[
https://issues.apache.org/jira/browse/PROTON-992?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14739069#comment-14739069
]
Alan Conway commented on PROTON-992:
------------------------------------
The general proton philosophy on thread safety is to be "thread agnosic". Calls
involving values associated with a single engine or reactor *cannot* be made
concurrently, but objects belonging to different engines or reactors are
independent and can be used concurrently simply because there are no shared
variables. Locking or serialization is left to higher layers, which is as it
should be. We need to preserve that philosophy.
The main problem is that proton tries to hide the sasl_client_init() and
sasl_client_done() calls in calls that can be made per-connection. Looking at
the cyrus code, these calls they are clearly not intended to be safe to call
multiple times or concurrently as they over-write global state. Many C
libraries have init/done functions that must be called exactly once per process
(from main() in C, from static ctor/dtor in C++ etc.) It is a hateful
requirement and proton has done well to avoid it but if we depend on external
libraries I fear we can avoid it no more. We need global init/done functions at
least for the sasl stuff. In a single threaded reactor we could call them as a
convenience in default handlers for reactor start/stop events but code using
multiple reactors in multiple thread contexts needs to be able to override such
behavior, and real C programmers will prefer to know that all the global
init/done functions are called in order under their control in main().
Practical experience with qpid C++ broker suggests that this is enough and
after that per-connection serialization is sufficient, qpidd does not set the
SASL mutex functions. However the SASL docs say it is *not* sufficient, and
maybe qpidd is just lucky or a future version of SASL will break it. We should
dig deeper before deciding what further serialization horrors need to be
inflicted on the per-connection part of the SASL negotiation.
> 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
> 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)
