[bt-devel] Our ftp transport backend

[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