Tentative fix proposal for thread crash after shared object unload

[bretambrose]

Resolved issues:

#3987

Description of changes:

s2n never clears the thread local slot associated with s2n_per_thread_rand_state_key which has a destructor callback. If s2n is unloaded before the callback is set to be invoked, a crash occurs when it is invoked.

This updates thread local cleanup of the random module to zero out the value associated with the s2n_per_thread_rand_state_key slot, preventing an invalid destructor callback when s2n was properly shut down before module unload.

Call-outs:

I am unsure if s2n_rand_cleanup_thread can be called while s2n_per_thread_rand_state_key is uninitialized. If that is the case, then error checking the result of pthread_setspecific might not be a good idea (if failure in turn disrupts succeeding cleanup).

Testing:

This change was tested by before-and-after runs of the small repro program in the original issue. A standalone test that reproduced that might be nice and if requested I could try and put that together as well.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[maddeleine]

A standalone test that reproduced that might be nice and if requested I could try and put that together as well.

Can you include a test for this fix? A unit test would help us ensure that we don't break this type of usecase in the future.

[bretambrose]

A standalone test that reproduced that might be nice and if requested I could try and put that together as well.

Can you include a test for this fix? A unit test would help us ensure that we don't break this type of usecase in the future.

I'm oncall this week so it may be a little while before I can get to it, but yes, I will try and integrate a test with the PR as soon as I can.

[bretambrose]

I've added a C-only test but it currently requires the path to the s2n shared object as a command line argument which leads to it never succeeding when run by CI. Running it manually with and without the fix produces the expected results. So we just need some kind of project/CI/control logic to only invoke this on a shared library build and give it the right path (or bake the path in to the exe if it's reliable).

[lrstewart]

Closing in favor of: #4024