[openssl-dev] [openssl-team] Discussion: design issue: async and -lpthread

[Nico Williams]

[Resend, with slight edits.]

[Viktor asked me for my advice on this issue and bounced me the post
 that I'm following up to.  -Nico]

The summary of what I've to say is that making libcrypto and libssl need
-lpthread is something that does require discussion, as it will have
detrimental effects on some users.  Personally, I think that those
detrimental effects are a good thing (see below), but nonetheless I
encourage you to discuss whether this is actually what OpenSSL should
do.  In particular, it may be possible to avoid -lpthread on some
systems and still get a subset of lipthread functionality from libc or
the compiler (e.g., thread-locals), and that may be worth doing.

On a slightly related note, I asked and Viktor tells me that fiber
stacks are allocated with malloc().  I would prefer that they were
allocated with mmap(), because then you get a guard page.  A guard page
would allow one to safely tune down fiber stack size to the whatever
OpenSSL actually needs for a given use.

Comments below.

On Mon, Nov 23, 2015 at 01:53:47AM +0000, Viktor Dukhovni wrote:
> As a side-effect of the MacOS/X MR I've become aware that the async
> code in its current state links master with "-lphtread", and defines
> macros that enable multi-theaded (as opposed to merely thread-safe)
> compilation into OpenSSL.
> 
>     commit 757d14905e3877abaa9f258f3dd0694ae3c7c270
>     Author: Matt Caswell <matt at openssl.org>
>     Date:   Thu Nov 19 14:55:09 2015 +0000
> 
> 	Add pthread support
> 
> 	The forthcoming async code needs to use pthread thread local variables. This
> 	updates the various Configurations to add the necessary flags. In many cases
> 	this is an educated guess as I don't have access to most of these
> 	environments! There is likely to be some tweaking needed.
> 
> 	Reviewed-by: Kurt Roeckx <kurt at openssl.org>
> 
> This is quite possibly not be the right thing to do, and deserves
> some attention from the team.  We might even seek outside some
> outside advice from folks well versed in platform-specific library
> engineering (Christos Zoulas from NetBSD, Nico Williams formerly
> from Sun, ...).
> 
> My concern is that introducing -lpthread automatically converts
> single-threaded applications that link with OpenSSL into threaded
> applications (with a single thread).  This may well have undesirable
> consequences.

Some background may be needed.  When threading was introduced in the
90s, and in some cases still to this day, generally the end result was
that the system had to support a number "process models", with potential
transitions from one to another at run-time:

 - single-threaded, dynamically linked
 - single-threaded, statically  linked
 - multi-threaded,  dynamically linked
 - multi-threaded,  statically  linked
 - multi-threaded,  mixed linkage

The statically-linked models can become mixed-linkage models via
dlopen().  The single-threaded model can become a threaded model via
dlopen() of an object linked with -lpthread, or by dlopen()ing
libpthread itself.

Solaris 9 and under used to have a veritable rats nest of code to deal
with the process model transitions from single-threaded to
multi-threaded.  Solaris 10 unified these and moved libpthread and libdl
into libc [with filters left behind for backwards compatibility].  Thus,
on Solaris 10 and up, and Illumos, OpenSSL using -lpthread or not makes
no difference.

I'm quite fond of the approach taken by Solaris 10 and up (and thus also
Illumos): there is but one process model, and it is threaded, with
pthreads in libc.  But that need not be the way it works everywhere.
Some systems may still support multiple process models.

For a library like OpenSSL making use of -lpthread does mean dictating
to users that they may only use a threaded process model.  OTOH, not
using -lpthread allows the user to choose a process model unconstrained
by such a library.

Until now OpenSSL has avoided forcing the user to choose any particular
process model.  Now with this commit OpenSSL is now taking the reverse
stance.  This seems like a very significant change that should at least
be noted prominently in the release notes, but it should also be
discussed, indeed.

Personally, I believe this change is a good thing, as OpenSSL really
ought to either automatically initialize its "lock callbacks" or do away
with them completely (leaving backwards compatibility stubs) and use the
OS' locking facility by default / only.  Automatic lock callback
initialization without forcing the use of -lpthread and still allowing
static linking would be tricky indeed (for example: OpenSSL couldn't use
weak symbols to detect when -lpthread gets brought in).

Still, if -lpthread avoidance were still desired, you'd have to find an
alternative to pthread_key_create(), pthread_getspecific(), and friends.

For thread-specifics the obvious answer is to use C compiler
thread-local variable support.  This might or might not be available;
this would have to be determined at build configuration time.  Still, if
where the compiler supports thread-locals, OpenSSL could avoid
-lpthread.

For pthread mutex functions (for lock callbacks) and, perhaps,
pthread_once() (for automatic initialization of lock callbacks,
perhaps),...  On some systems these functions will be in libc.  For the
rest it's very difficult to create alternatives to requiring -lpthread,
though it can conceivably be done within a library like OpenSSL.  I just
don't think it's worth doing though!  Instead I think it's best to just
use -lpthread and let the OS and its libc and libpthread take care of
any desirable optimizations in single-threaded cases, as well as any
automatic transitions to non-optimized implementations upon creation of
the first additional thread.

The single-threaded and statically linked process model has a number of
detrimental effects, and ISTM that by bending over backwards to avoid
forcing the choice of process model, OpenSSL and similar allow users to
believe in fairy tales.  One detrimental effect has been OpenSSL's
bloody lock callbacks, which in turn mean that it is impossible to use
OpenSSL thread-safely from another library!  Must every program use
-lcrypto and -lpthread, and setup lock callbacks just in case some other
library the program really does use calls dlopen()?

Other detrimental effects of static linking include:

 - the flattened dependency topology forced by static linking (with
   attendant confusion in *-config(1) programs), with attendant
   accidental symbol interposition;

 - partial loss of ASLR protection (one random load address for the
   entire executable text and all its libraries, versus one for each
   object)

 - the need for complex single- to multi-threaded transition support
   code in various system libraries (mostly in libc).

 - slower load times.  Yes, slower.  A single statically-linked
   executable will load faster than the same program dynamically-linked.
   But a larger system than just one executable (e.g., the host OS,
   related applications, helper programs, ...) will load slower because
   multiple copies of common texts will not be sharable.

If a major library like OpenSSL were to force operating systems to deal
with the shortcomings of mixed process models and static linking, that
would be a good thing.  That's what OpenSSL would be doing by using
-lpthread.

> The C-library on many platforms is carefully designed to operate
> efficiently and lock-less-ly in single-threaded applications, and to
> only switch to thread-aware implementations of functions when the
> pthread library is loaded.

I don't think the loss of the lock-less optimizations should be of
particular concern to OpenSSL: if that's a problem for a given OS, let
its implementors deal with it.  (They can do it, by, e.g., hot-swapping
lock functions at pthread_create() call time rather than at the time
that libpthread is loaded.)

> In NetBSD, for example, the <pthread.h> header file carefully
> defines non-threaded (or rather lazily threaded only when pthread
> is also loaded) versions of various pthread functions, see the
> extract of pthread.h below my signature.

It shouldn't be the loading of libpthread that causes transition to a
threaded model (and, therefore, loss of single-threaded optimizations),
but calling pthread_create().  If NetBSD really does the former, well,
too bad: it really ought to be changed to do the latter.

> Therefore, on any platform where the C-library is properly engineered
> in this way (at least NetBSD and MacOS/X), we should not be
> introducing -lpthread or definining "-D_REENTRANT", ...

Nonetheless, this is good advice.  If the necessary functionality from
libpthread can be found in libc (or, in the case of thread-locals, in
the compiler), then OpenSSL should avoid bringing in -lpthread.

As long as they support seamless upgrade to threaded model via dlopen(),
this is good advice.

Of course, this does mean that OpenSSL needs more complexity in its
configurator: to decide when to use -lpthread and when not to.

> We are a thread-safe library, *not* a threaded library, and must
> not introduce threading into threaded applications.

That's a bit of a fairy tale.

OpenSSL is not thread-safe when called from other libraries, as a) it's
not their place to initialize OpenSSL's lock callbacks, b) the program
might not do it either, c) OpenSSL defaults do no locking, d) the lock
callback initialization interfaces assume no races to initialize lock
callbacks, which means that other libraries can't safely initialize them
(in practice one library is likely to always win the race, still).

It's time to fix this.  If OpenSSL will now require -lpthread, then it
gets easier, certainly, so I'm not exactly against OpenSSL requiring
-lpthread!  ;)

> We should think carefully about which platforms get async support
> (perhaps one at time, and before it has been tested), and whether
> "-pthread" is an acceptable penalty for any potential future
> advantage of async support (none at present).
> 
> Thoughts?  Comments?

See above.  I'm not sure you'll be please with mine :)

> -- 
> 	Viktor.
> 
> [NetBSD header commentary extracts elided]

Nico
--