On Thu, Jan 11, 2001 at 01:34:07PM +0000, Dr S N Henson wrote:
>>>> + o crypto/ex_data.c is not really thread-safe and so must be used
>>>> + with care (e.g., extra locking where necessary, or don't call
>>>> + CRYPTO_get_ex_new_index once multiple threads exist).
>>>> + The current API is not suitable for everything that it pretends
>>>> + to offer.
>>> Can you expand on this so we can decide what to do about it?
I've moved this from "NEEDS PATCH" to "OPEN ISSUES" by now. Probably
the behaviour of this is no longer buggy in the technical sense if
sufficient care is taken w/r/t multi-threading; it's just utterly
horrible. We should have something that works by design, not
something that just happens to work because the idiosyncracies of its
various parts can be made compatible with one another by adding
some locking here and there ...
>> There has already been some discussion on this; see follow-ups to my
>> commits in ex_data.c, e.g. this message:
> Yes I saw those I wondered if you saw any other issues?
No, nothing new.
>> Of course you need *some* way to add application-defined data to about
>> everything, but it definitely should not look like ex_data.c.
>> If all flavours of extra pointers (the ex_data indexes with their
>> associated new_funcs, dup_funcs, and free_funcs) were always allocated
>> at application initialization before additional threads are spawned,
>> then everything should be well; but this is not how ssl/ssl_cert.c
>> works. Unless we declare that ssl_cert.c is wrong to use ex_data in a
>> possibly multi-threaded environment, we have to blame ex_data.c for
>> using global data structures (ex_data "meth"s) in an inappropriate
>> way, and also overusing them in that concurrency would be severely
>> limited because of the locking requirements if ex_data.c were made
>> thread-safe (dynlocks can't help here because the "meth"s are global).
> So we give the advice that applications should call the
> get_ex_new_index() before starting any threads. In this regard ex_data
> is no worse than other things such as the object or algorithm tables
> which can get in a mess if applications start adding the same object or
> algorithm in multiple threads.
>
> Now ssl/ssl_cert.c does indeed have to do something different because it
> can have problems if it is called in multiple threads. [...]
> That by itself doesn't violate the advice since we're saying
> applications shouldn't call get_ex_new_index() in multiple threads and
> often have to do things applications shouldn't in the library itself.
So I guess what we should do is move the X509_STORE_CTX_get_ex_new_index()
call into SSL_library_init (which must be called before there are
multiple threads that might interfere with each other), store the
return value in a static variable, and use only the value from this
variable in potentially multi-threaded functions.
--
Bodo Möller <[EMAIL PROTECTED]>
PGP http://www.informatik.tu-darmstadt.de/TI/Mitarbeiter/moeller/0x36d2c658.html
* TU Darmstadt, Theoretische Informatik, Alexanderstr. 10, D-64283 Darmstadt
* Tel. +49-6151-16-6628, Fax +49-6151-16-6036
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
