Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Reworking cleanup behavior #4871

Merged
merged 14 commits into from
Nov 12, 2024
Merged

feat: Reworking cleanup behavior #4871

merged 14 commits into from
Nov 12, 2024

Conversation

maddeleine
Copy link
Contributor

@maddeleine maddeleine commented Nov 4, 2024

Resolved issues:

N/A

Description of changes:

This change disables the atexit handler by default in our library.

Call-outs:

I am not disabling the atexit handler in our unit tests. If I remove it there then we get a huge amount of memory leaks and then PR to fix them becomes gigantic.

Testing:

CI should pass

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

@github-actions github-actions bot added the s2n-core team label Nov 4, 2024
@maddeleine maddeleine force-pushed the cleanup branch 2 times, most recently from 0392a51 to 6d23b13 Compare November 7, 2024 21:35
* s2n_cleanup will only perform a complete cleanup if the atexit handler
* is disabled and s2n_cleanup is called by the thread that called s2n_init.
* Therefore s2n_cleanup_final should be used instead of s2n_cleanup in cases
* where the user needs full control over when the complete cleanup executes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am deleting this paragraph because s2n_cleanup no longer performs a full cleanup so most of this information is incorrect.

utils/s2n_random.c Outdated Show resolved Hide resolved
tests/unit/s2n_init_test.c Show resolved Hide resolved
tests/unit/s2n_init_test.c Outdated Show resolved Hide resolved
docs/usage-guide/topics/ch02-initialization.md Outdated Show resolved Hide resolved
utils/s2n_random.c Outdated Show resolved Hide resolved
utils/s2n_init.c Show resolved Hide resolved
codebuild/bin/s2n_dynamic_load_test.c Outdated Show resolved Hide resolved
docs/usage-guide/topics/ch02-initialization.md Outdated Show resolved Hide resolved
docs/usage-guide/topics/ch02-initialization.md Outdated Show resolved Hide resolved
docs/usage-guide/topics/ch02-initialization.md Outdated Show resolved Hide resolved
docs/usage-guide/topics/ch02-initialization.md Outdated Show resolved Hide resolved
docs/usage-guide/topics/ch02-initialization.md Outdated Show resolved Hide resolved
@maddeleine maddeleine enabled auto-merge (squash) November 12, 2024 18:18
@maddeleine maddeleine merged commit 493b771 into aws:main Nov 12, 2024
37 checks passed
@maddeleine maddeleine deleted the cleanup branch November 12, 2024 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants