[sword-devel] curl library and download termination

[Troy A. Griffitts]

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