[bt-devel] Our ftp transport backend

Sun Nov 1 10:01:53 MST 2009

Eeli,

I agree with all your arguments. If the problems can be solved inside Sword, 
that would be fine. Your arguments for a Qt-based solution are still valid, 
though, and Sword does have the dedicated interface to allow replacing the 
transport backend with a custom one - so it would be more than acceptable to 
also use it.

mg

Am Sonntag, 1. November 2009 17:39:10 schrieb Eeli Kaikkonen:
> For those who don't read sword-devel I'll append the discussion with
> Troy.
> 
> I've been planning to write an ftp transport backend with Qt networking
> classes. I don't have any working code but I have a design which
> "should" work. the Qt code itself is very simple to use but the class
> structure and communication must be adapted because SWORD uses
> synchronous method and Qt asynchronous.
> 
> There were originally three reasons for our own backend. The most
> important is that the SWORD implementation isn't thread safe. It may be
> one reason why we have random crashes. Even though they were mostly
> eliminated by turning the parallel downloads off they seem to be not
> gone 100%.
> 
> The second reason is interwined with the first one. Cancelling a
> download doesn't work in the current SWORD implementation and therefore
> I have tried to terminate it violently. But it's a hack which probably
> doesn't behave well. That may be another reason for crashes.
> 
> The third reason is that if we had a Qt implementation we could drop off
> the curl dependency in Windows. I don't know if the fallback ftplib is
> used in any platform where SWORD is packaged, but that surely won't work
> for BibleTime, so it must be curl or Qt.
> 
> I'd like to hear what BT developers think. Most of you probably don't
> have any strong feelings, but is there anything which should be taken
> into consideration?
> 
> The changes proposed below should be quite easy to implement, so I'll
> try to look at them deeper and test them anyways. If they work,
> it could be possible to keep using the current code. But would there
> then be any reasons to implement our own backend with Qt?
> 
>   Yours,
> 	Eeli Kaikkonen (Mr.), Oulu, Finland
> 	e-mail: eekaikko at mailx.studentx.oulux.fix (with no x)
> 
> ---------- Forwarded message ----------
> Date: Fri, 30 Oct 2009 18:27:32 -0700
> From: Troy A. Griffitts <scribe at crosswire.org>
> Reply-To: SWORD Developers' Collaboration Forum <sword-devel at crosswire.org>
> To: SWORD Developers' Collaboration Forum <sword-devel at crosswire.org>
> Subject: Re: [sword-devel] curl library and download termination
> 
> Thanks for the detailed analysis Eeli-- most appreciated.  If you have a
> chance to make the changes you suggest and determine they work better
> for you, I'd be happy to commit the delta.  Do you think this is
> something we can do without changing public class definitions, so we can
> keep it in the 1.6.x thread?  Looking briefly, I suppose we could:
> 
> in getURL:
> - curl_easy_setopt(session, CURLOPT_PROGRESSDATA, statusReporter);
> + curl_easy_setopt(session, CURLOPT_PROGRESSDATA, this);
> 
> in my_fprogress:
> - ((StatusReporter *)clientp)->statusUpdate(dltotal, dlnow);
> + ((CURLFTPTransport *)clientp)->statusReporter->statusUpdate(dltotal,
> dlnow);
> + return ((CURLFTPTransport *)clientp)->term;
> 
> 
> plus your suggestion of calling curl_global_init
> 
> 
> Anyway, let me know.  It sounds like you have investigated the correct
> way to do CURL more than anyone, so I'm happy for an update!
> 
> 	-Troy.
> 
> Eeli Kaikkonen wrote:
> > Currently the install manager ftp transport curl implementation doesn't
> > correctly terminate the file transport. It blocks until the ongoing file
> > download has been completed. It's of course very bad for usability -
> > think of a module with large images and a slow connection. The whole
> > user interface could be stalled for minutes and the only way to cancel
> > downloading would be to terminate the whole program.
> >
> > I think this could be fixed quite easily. Curl supports termination with
> > CURLOPT_PROGRESSFUNCTION which is set in sword curlftpt.cpp. The sword
> > implementation of this callbackfunction returns always 0, but if it
> > would return !=0, the curl download would terminate (within about 1
> > second). The sword class already has the term member data which is set
> > by the instmgr, but it's not used in the transport implementation.
> >
> > curl_easy_perform returns an error code which is is set to a certain
> > value. I'm not sure if this should be taken into consideration. But
> > there shouldn't be anything else to change.
> >
> > BibleTime has also a more serious issue with the curlftpt
> > implementation.  When I earlier looked into curl docs I thought that the
> > "easy" functions are not thread safe. However, it looks like I was
> > wrong, but sword implemenation calls the library in a way which is not
> > thread safe.
> >
> > The curl doc about curl_easy_init:
> >
> > "If you did not already call curl_global_init(3), curl_easy_init(3) does
> > it automatically. This may be lethal in multi-threaded cases, since
> > curl_global_init(3) is not thread-safe, and it may result in resource
> > problems because there is no corresponding cleanup.
> > You are strongly advised to not allow this automatic behaviour, by
> > calling curl_global_init(3) yourself properly."
> >
> > It shouldn't be too hard to replace per-object easy_init with static
> > global_init.
> >
> > This thread safety problem with the download termination problem have
> > caused crashes in BibleTime. I'm planning to implement our own transport
> > with the Qt library, but if these problems are fixed, we might go on
> > with the default sword implementation.
> >
> >   Yours,
> > 	Eeli Kaikkonen (Mr.), Oulu, Finland
> > 	e-mail: eekaikko at mailx.studentx.oulux.fix (with no x)
> >
> > _______________________________________________
> > sword-devel mailing list: sword-devel at crosswire.org
> > http://www.crosswire.org/mailman/listinfo/sword-devel
> > Instructions to unsubscribe/change your settings at above page
> 
> _______________________________________________
> sword-devel mailing list: sword-devel at crosswire.org
> http://www.crosswire.org/mailman/listinfo/sword-devel
> Instructions to unsubscribe/change your settings at above page
> 
> _______________________________________________
> bt-devel mailing list
> bt-devel at crosswire.org
> http://www.crosswire.org/mailman/listinfo/bt-devel
> 

-- 

Bauplan des Lebens - längst im Gen entdeckt!
Die Wissenschaft ist stolz: Sie hat's gecheckt.
Nun ist der Bauplan als Beweis beliebt,
dass es den Architekten gar nicht gibt.

Wolf Rahn