[gnutls-dev] Re: living without global variables

[Simon Josefsson]

bryanh at giraffe-data.com (Bryan Henderson) writes:

>>It is the ASN.1 schema for various PKCS and PKIX standards, in a
>>compiled form for libtasn1.
>
> I have no idea what those words mean, but I guess I don't have to.

Regard them as read-only data structures.  The variables are used by
code that has to parse X.509 certificates or other PKCS structures.
Several of those functions doesn't have a gnutls_session parameter.

>>Perhaps GnuTLS should deallocate these variables in an `atexit' hook
>>instead.  Then it seem all of this problem would go away.  In GNU
>>libc, atexit hooks are even called if GnuTLS was dlopen+dlclose'd.
>>gnutls_global_deinit() would then only call gc_done().
>
> If you're willing to do that, you should be willing just to skip the
> deallocate altogether.  On process termination, memory allocation
> becomes irrelevant.  Most engineers I know prefer to have more
> modularity -- the library doesn't know about process termination or
> program lifetime; it has a matching setup and shutdown and after
> shutdown, everything is back the way it was before setup.  But this
> would be acceptable to me (acceptable means I would still use GnuTLS).
>
> You know, I've seen the code of dozens of Unix libraries and hundreds
> of Unix programs, and I don't think I've ever seen atexit() used.  And
> while I've been reminded of its existence from time to time, I've
> never been able to come with a use for it in my own code.

I was merely suggesting it as a solution, and as Freddy pointed out,
using atexit in a library is probably a bad idea, so I agree with you.

>>That is a memory leak, but a pretty insignificant one, and one
>>that would happen rarely.  There is no serious problem, that cannot
>>easily be solved by the caller, as far as I can see.  Nothing else bad
>>will happen, as far as I can see.
>
> You're probably right, though I prefer not to attempt analyses of this
> kind.  It's easier just to say it's thread safe or it's not.  In this
> case, it's not, and you're pointing out that the user can tolerate
> that as he does any non-thread-safe library -- make sure he doesn't
> call it simultaneously from two different threads.  I also agree that
> the damage is slight and very unlikely, and a user may be willing just
> to sweep it under the carpet.  But I've never seen anyone take the
> position that just a little thread unsafety or a little memory leak is
> OK in published code -- computer people are always interested in
> precision.

If you want precision, gnutls_global_init() is not thread safe.

>>Alternatively, each thread could do a mutex around the call to
>>gnutls_global_init:
>
> And that's the nonmodularity I'm trying to get around.  Each thread has
> to be aware that other modules in other threads might be using the same
> library and agree with them on a locking protocol.

I think you need to show us how to do the things you describe.  I
don't see a way to make things better than they are now.  I believe
libgcrypt initialization has a similar same race-condition.

>>For example, many libraries has chosen to only support one thread
>>library, and then use mutexes from that thread library to solve
>>problems like this.  GnuTLS could have done that too, and wrapped
>>gnutls_global_init and gnutls_global_deinit around thread mutexes.
>>But I believe that is too inflexible for GnuTLS, consider this:
>
> Yes, I agree.
>
> There are only two methods I've seen to make a stateful library thread-safe:
>
>   1) the library routines use locks
>
>   2) the library routines each take a complete context as an argument
>      (usually thought of as the object oriented approach).
>
> I prefer (2).

There is another popular approach:

3) Declare functions thread-unsafe, and let the caller handle the
   thread issue.  This approach is used by many POSIX and libc
   functions.

Another possibility is:

4) Store the global variables in thread-local storage.  There is a
   gnulib module that works with pthread, pth, solaris threads and
   win32 threads to do this.

>>Read-only constant global variables are not a serious problem as far
>>as I know.
>
> These aren't read only, of course -- they get set twice.

I mean read-only except in gnutls_global_init and
gnutls_global_deinit.  Those two functions are not thread safe, and
were not intended to be (hence "global").

Doesn't your library has a global initialization method?

> But the exemption still holds as long as a global variable is
> repeatably settable.  For example,
>
>    static struct {
>       int version;
>       int maxConnections;
>    } globalStuff;
>
>    void
>    global_init() {
>      globalStuff.version = 3;
>      globalStuff.maxConnections = 1024;
>    }
>
> is acceptable as an alternative to having the loader set those
> constants with a C initial value declaration.  But when there's a
> malloc involved, you don't have the repeatability (i.e. subsequent
> sets are not idempotent).
>
> What I haven't seen so far in all the fishing for an alternative is
> what is the drawback to adding the 3 functions to allow a thread to
> have totally private context if it wants?

Someone has to write the patch, and test it, and I proposed something
that I thought would be simpler.  If you write the patch, we can look
at it, and if it works, and doesn't have other problems, we could use
it.  However, remember that gnutls_global_init would still be thread
unsafe, since libgcrypt initialization is thread-unsafe, so your
problem would persist.  I am trying to rewrite the crypto engine so
that we can use crypto modules from gnulib, to avoid libgcrypt, to fix
that problem.  Help on doing that would also be appreciated, and might
solve your concerns in the long run.

Thanks,
Simon