← Back to team overview

kicad-developers team mailing list archive

Re: libcurl github race condition

 

On 1/10/2016 7:49 PM, Mark Roszko wrote:
> Just a heads up as I fix this, there's a race condition with how
> libcurl in KiCad is implemented on Windows involving openssl.
> 
> 
> Turns out openssl being as trashy as it is has all kinds of thread
> safety issues without linking against it directly and doing something
> stupid (use its own locking functions) at thread level. I don't want
> to know why they have issues even doing a sha512 hash separately in
> threads, it's just ridiculous.

This appears to be well known and documented.

http://www.openssl.org/docs/manmaster/crypto/threads.html

Apparently avhttp was doing this properly behind the scenes.  Blaming
this on openssl is not entirely accurate.  OpenSSL was used with avhttp,
and claims it is thread safe if you provide the functions needed to
support multi-threading from the client.

The requirements for these functions are documented in libcurl, which
even has an example in their source tree.

http://curl.haxx.se/libcurl/c/opensslthreadlock.html

This is a perfectly reasonable design, because it gives openssl the
means of being cross platform, by moving the locking mechanisms up into
the client.  In other words, if you want to use multiple threads, it is
up to you to provide the locking mechanisms shown in the example above.

> 
> 
> OSX which no longer uses openssl isn't affected. Linux distros with
> openssl based libcurl are affected but theres many distros now using
> ntls or gnutls in place of openssl. So only windows users may be
> getting an ~occasional crash.~

I would not assume that these issues don't exist when libcurl is build
with ntls or gnutls.  This will require additional testing when libcurl
is build with them.

> 
> 
> 
> My proposed solution:
> 1. Toss concurrent downloading out the window. It's a horrible idea.
> No package manager does this (apt, yum, Steam, Windows Update, etc).
> You risk tripping firewalls and just split your bandwidth over
> multiple connections. You can even peg your CPU with the https
> overhead attempting to do it simultaneously on 40 separate
> connections. Actually on that note, they'll block each other if you
> exist your system entropy on Linux momentarily.

I do not share your optimism about the performance hit.  It's easy
enough to test.  Set READER_THREADS in common/footprint_info.cpp to 1
and you now have a single thread github access which is what you are
proposing.  You will see dramatic increase in the amount of time it
takes to load the libraries in cvpcb.  If you don't believe me, try it
for yourself.

I'm assuming you are using hyperbole when you say 40 separate
connections because even before Dick's last patch the maximum number of
threads was 13 so 40 separate connections would not have been possible
unless there was a bug in the multiple thread code.  I try to avoid
using hyperbole in dev emails to prevent other readers from believing
that it's actually the case.  Hyperbole makes for great fiction.  No so
much for general communication.  The current number of threads is 6 so
there should be not more than 6 simultaneous connections.

> 
> 2. Add a "update progress" UI, its completely stupid that when you
> open CVPCB that it silently tries to update your 30+ libraries in the
> background with no indication. This is really bad on users on bad
> connections.

I agree that there should be some progress indicator.  Even multiple
threaded access may require some type of indicator on slow connections.

I'm very much starting to regret the decision to commit the libcurl
changes without more testing.  I allowed my frustration with the avhttp
project regarding their handling of a patch that I sent them to cloud my
judgement on this decision.  Shame on me and lesson learned.  If we
can't resolve the libcurl in a reasonable time frame, I'm seriously
considering falling back to avhttp which was less problematic.  Please
keep me informed about the progress on fixing this.

Cheers,

Wayne

> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
> 


Follow ups

References