qpid-proton mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alan Conway (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (PROTON-992) Proton's use of Cyrus SASL is not thread-safe.
Date Fri, 18 Sep 2015 14:02:04 GMT

    [ 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)

Mime
View raw message