Add sodium_finalize()

[cmb69]

sodium_crit_enter() lazily initializes a critical section on Windows but that critical section is never deleted (DeleteCriticalSection()). This is not an issue per se, since the critical section is released on process termination, but Application Verifier complains about that ("Unloading DLL containing an active critical section.")

There is no way yet to delete this critical section, since it is held in a file static variable, and there is no API to delete it. Thus, I suggest to introduce sodium_finalize() which can be called by clients to delete the critical section on process termination.

The new API should be available on all supported platforms, although I don't know whether similar issues exists elsewhere. For now, it does nothing on other platforms than Windows.

---

For PHP's sodium extension, it could be used like:

 ext/sodium/libsodium.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ext/sodium/libsodium.c b/ext/sodium/libsodium.c
index abde3b8994..2eefd569a1 100644
--- a/ext/sodium/libsodium.c
+++ b/ext/sodium/libsodium.c
@@ -655,6 +655,7 @@ PHP_MINIT_FUNCTION(sodium)
 PHP_MSHUTDOWN_FUNCTION(sodium)
 {
    randombytes_close();
+   sodium_finalize();
    return SUCCESS;
 }

[jedisct1]

We can't really expect applications to call a deinit function at this point.

This change is also unsafe: the critical section can be destroyed while _sodium_lock_initialized is still set.

And this is probably not be necessary (see https://github.com/jedisct1/libsodium/issues/1050#issuecomment-817865691 for a different approach).

[jedisct1]

For DLLs, the DllMain() function is apparently called with a DLL is unloaded: https://docs.microsoft.com/en-us/windows/win32/dlls/dllmain

That may be another way to do it without requiring applications to call a new function.

[cmb69]

We can't really expect applications to call a deinit function at this point.

As it is now (with this PR), there would be no need to call a deint function. :)

Deleting the critical section in sodium_crit_leave() appears to be inefficient (would require to initialize it over and over again).

I thought about DllMain() as well, but I'm not sure if there won't be issues, because there is a long aritcle about what not to do in DllMain().

Anyhow, thanks for your swift reply! I'll have a closer look at doing the deletion in DllMain(), and update the PR respectively.

[jedisct1]

It not being super efficient shouldn't be an issue. This is only used to protect against multiple parallel executions of sodium_init().

[cmb69]

It not being super efficient shouldn't be an issue. This is only used to protect against multiple parallel executions of sodium_init().

Ah, right! Anyhow, just submitted PR #1058, which might be preferable.