[ 
https://issues.apache.org/jira/browse/PROTON-992?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14875651#comment-14875651
 ] 

Alan Conway commented on PROTON-992:
------------------------------------

You need more than an atomic counter to implement exactly once - if there are 
concurrent calls to init you must not only ensure that only one of them 
executes, but also that the others *wait till it is complete*. You need a lock 
or the equivalent of pthread_once. In terms of safety, ease of use and avoiding 
API change I agree this is the way to go. However my concern is that it makes 
proton harder to port since we need a portable abstraction for "once" and we 
introduce a dependency on some library for locking. The original design goal of 
proton was to be a very portable, pure C99 codebase, thread *agnostic* 
codebase. Pulling in a dependency on pthreads doesn't seem to be in that 
spirit. E.g. I'm not sure how pthreads will play with the Go binding. That's 
why the SASL allows you to specify locks externally - so it doesn't force a 
thread library on you. That's a horrible solution but I'm not sure I like 
forcing pthreads any better.

The "traditional" C solution to this problem is to have sufficient start-up and 
shut-down functions that must be called in main before and after use or yes, 
you will get leaks. It's C. Everybody does it - SASL, OpenSSL, everybody. 
Trying to be more clever than the language we are coding in seems likely to 
lead to trouble - IMO it already has and this JIRA is it.

> Proton's use of Cyrus SASL is not thread-safe.
> ----------------------------------------------
>
>                 Key: PROTON-992
>                 URL: https://issues.apache.org/jira/browse/PROTON-992
>             Project: Qpid Proton
>          Issue Type: Bug
>          Components: proton-c
>    Affects Versions: 0.10
>            Reporter: michael goulish
>            Assignee: michael goulish
>            Priority: Critical
>
> Documentation for the Cyrus SASL library says that the library is believed to 
> be thread-safe only if the code that uses it meets several requirements.
> The requirements are:
>     * you supply mutex functions (see sasl_set_mutex())
>     * you make no libsasl calls until sasl_client/server_init() completes
>     * no libsasl calls are made after sasl_done() is begun
>     * when using GSSAPI, you use a thread-safe GSS / Kerberos 5 library.
> It says explicitly that that sasl_set* calls are not thread safe, since they 
> set global state.
> The proton library makes calls to sasl_set* functions in :
>           pni_init_client()
>           pni_init_server(), and
>           pni_process_init()
> Since those are internal functions, there is no way for code that uses Proton 
> to lock around those calls.
> I think proton needs a new API call to let applications call 
> sasl_set_mutex().  Or something.
> We probably also need other protections to meet the other requirements 
> specified in the Cyrus documentation (and quoted above).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to