On 1. 7. 25 10:39, dsahlberg-apache-org (via GitHub) wrote:
dsahlberg-apache-org commented on code in PR #8:
URL:https://github.com/apache/serf/pull/8#discussion_r2160333222


##########
buckets/ssl_buckets.c:
##########
@@ -1515,6 +1534,12 @@ static void init_ssl_libraries(void)
          OpenSSL_add_all_algorithms();
  #endif
+#if defined(SERF_HAVE_OSSL_STORE_OPEN_EX)
+        if (ssl_x509_ex_data_idx < 0) {
+            ssl_x509_ex_data_idx = X509_get_ex_new_index(0, NULL, NULL, NULL, 
NULL);
+        }
+#endif
+

Review Comment:
    This looks good to me now, even if X509_get_ex_new_index() isn't thread 
safe, have_init_ssl should make sure it can't be run by several threads at the 
same time.
However, do we need to CRYPTO_free_ex_index() the index, for example in cleanup_ssl?

You won't even have an ssl_pool now with OpenSSL 3. Because SERF_HAVE_SSL_LOCKING_CALLBACKS is not defined now that we finally build with -DOPENSSL_NO_DEPRECATED, so all those callbacks and the init code that creates the pool just vanish into thin air.

Note, that's for the CMake build; I've not added this option in SCons yet, and I'm sort of too saturated with all the build scripts to do it any time soon.

In any case, even ignoring that, the pool only gets created if APR_HAS_THREADS. If we need cleanup for that, the whole init logic needs some grooming.

One would guess that no-one sane would ever unload libserf.so, but stranger things have happened...

-- Brane

Reply via email to