Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23222 )
Change subject: KUDU-3635 call OPENSSL_cleanup() explicitly ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/23222/3/src/kudu/master/master_main.cc File src/kudu/master/master_main.cc: http://gerrit.cloudera.org:8080/#/c/23222/3/src/kudu/master/master_main.cc@57 PS3, Line 57: void module_init_tcmalloc() { : #if defined(TCMALLOC_ENABLED) : // To prompt tcmalloc's initialization (if hasn't happened yet), : // call one of its functions. : kudu::process_memory::GetTCMallocCurrentAllocatedBytes(); : #endif : } : : void module_fini_tcmalloc() { : #if defined(TCMALLOC_ENABLED) && !defined(NDEBUG) : // Debug builds only: as an extra sanity check, poke tcmalloc a bit to make : // sure it's not erroring out at this point. : RAW_VLOG(2, "invoking tcmalloc GC"); : kudu::process_memory::GcTcmalloc(); : #endif : } : : void module_init_openssl() { : // In debug builds, make sure OpenSSL isn't yet initialized at this point. : RAW_DCHECK(!kudu::security::IsOpenSSLInitialized(), : "OpenSSL shouldn't be initialized yet"); : } : : void module_fini_openssl() { : // Call OPENSSL_cleanup() to release resources and clean up the global state : // of the library (applicable to OpenSSL 1.1.0 and newer versions). : RAW_VLOG(2, "cleaning up OpenSSL runtime"); : kudu::security::FinalizeOpenSSL(); : } > I see that these functions occur in all the *main.cc files. Would it be pos Exactly my thoughts -- I was unsure what might be the best option here, and tried a few. The only viable option for this is a textual include, making sure it's the same translation unit. Done. http://gerrit.cloudera.org:8080/#/c/23222/3/src/kudu/util/entry_exit_wrapper.h File src/kudu/util/entry_exit_wrapper.h: http://gerrit.cloudera.org:8080/#/c/23222/3/src/kudu/util/entry_exit_wrapper.h@32 PS3, Line 32: reserved reverse http://gerrit.cloudera.org:8080/#/c/23222/3/src/kudu/util/openssl_util.cc File src/kudu/util/openssl_util.cc: http://gerrit.cloudera.org:8080/#/c/23222/3/src/kudu/util/openssl_util.cc@193 PS3, Line 193: " > Do we want to add more details on the error here? Something from ERR_get_er I'd love to, but in reality it's not feasible when OPENSSL_init_ssl() returns non-OK status here. That's because the whole error subsystem (error strings, error stack, etc.) might not be initialized. Trying to access error stack might lead to SIGSEGV with unexpected stack trace, and I'm not sure it's better than the expected SIGABRT with stack trace that's easy to reason about. And the non-OK status is always 0 -- it doesn't discriminate between different errors, at least per the OpenSSL docs. http://gerrit.cloudera.org:8080/#/c/23222/3/src/kudu/util/openssl_util.cc@247 PS3, Line 247: #if !defined(DEBUG) This should have been #ifdef(NDEBUG) http://gerrit.cloudera.org:8080/#/c/23222/3/src/kudu/util/openssl_util.cc@254 PS3, Line 254: 0x10100000L > nit: shouldn't this be 0x10101000L? This is a good catch! Indeed, that should have been 0x10101000L. Fixed. -- To view, visit http://gerrit.cloudera.org:8080/23222 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib08310d66a7eabb1996bde901f39f36f54bff483 Gerrit-Change-Number: 23222 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Sat, 02 Aug 2025 02:14:55 +0000 Gerrit-HasComments: Yes
