Skip to content

Disable atexit hook for OpenSSL#80138

Merged
thevar1able merged 1 commit intomasterfrom
openssl-no-atexit
May 13, 2025
Merged

Disable atexit hook for OpenSSL#80138
thevar1able merged 1 commit intomasterfrom
openssl-no-atexit

Conversation

@thevar1able
Copy link
Copy Markdown
Member

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Closes #80132

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented May 12, 2025

Workflow [PR], commit [1eac616]

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label May 12, 2025
@alexey-milovidov alexey-milovidov self-assigned this May 12, 2025
@thevar1able thevar1able enabled auto-merge May 13, 2025 02:08
@thevar1able thevar1able added this pull request to the merge queue May 13, 2025
Merged via the queue into master with commit 4a23e77 May 13, 2025
120 checks passed
@thevar1able thevar1able deleted the openssl-no-atexit branch May 13, 2025 02:20
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-synced-to-cloud The PR is synced to the cloud repo label May 13, 2025
#if USE_SSL
if (ref_count++ == 0)
{
// Disable OpenSSL atexit hook.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I read #80132 correctly, then we crashed when the atexit handler is installed, not when it runs at the end of the program. Also, https://docs.openssl.org/3.2/man3/OPENSSL_init_crypto/#description says that `OPENSSL_INIT_NO_ATEXIT By default OpenSSL will attempt to clean itself up when the process exits via an "atexit" handler. Using this option suppresses that behaviour. This means that the application will have to clean up OpenSSL explicitly using OPENSSL_cleanup().

My question is if we should call OPENSSL_cleanup in the dtor?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we cleanup on shutdown?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OPENSSL_cleanup can be called only once according to their docs, so yes: we ideally call it during database shutdown and not here (dtor).

@azat
Copy link
Copy Markdown
Member

azat commented May 29, 2025

@robot-clickhouse robot-clickhouse added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label May 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-not-for-changelog This PR should not be mentioned in the changelog pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_host_regexp_multiple_ptr_records causes race in OpenSSL OPENSSL_cleanup

6 participants