-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Add hooks to debug OpenSSL memory allocations #111539
Open
rzikm
wants to merge
21
commits into
dotnet:main
Choose a base branch
from
rzikm:openssl-mallloc-debug-hooks
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+339
−2
Open
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
61d9b38
add hooks to debug OpenSSL memory
wfurt ae0b47b
opensslshim
wfurt 3de18d1
1.x
wfurt 32b5b22
1.0.1
wfurt a5ceb86
android
wfurt 04819ad
Collections
wfurt a810556
unsafe
wfurt 83795f2
build
wfurt 04f5bde
feedback
wfurt d9bc610
update
wfurt de0ef7d
feedback
wfurt c0d8f2d
feedback
wfurt 250822d
feedback
wfurt 607b4d7
fix build
wfurt e29aeb1
gcc
wfurt 518446c
gcc
wfurt 860490a
Update src/native/libs/System.Security.Cryptography.Native/openssl.c
wfurt ebcb3d8
Move init to Interop.Crypto
rzikm 918d337
Fix data race on GetIncrementalAllocations
rzikm 23af46f
Use modern tuple type
rzikm 36cd612
Fix typo
rzikm File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling managed code from the free method is going to have reliability issues. The problem is that OpenSSL can call free very late during thread destruction after the .NET runtime cleaned up its own per-thread data structures. It will cause the per-thread .NET runtime data structures to be partially reinitialized and stuck in a weird state. This weird state will manifest as memory leak and a hard to diagnose crash later.
Take a look at #110442 to see the crash caused by calling back managed code from OpenSSL malloc/free and what it took to diagnose it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there some way to know if runtime or thread is going down @jkotas? I don't envision this to be enabled often or in production and we can also document the dragons ... or keep it hidden and make it debug hook for us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think there is a reliable way to detect that the thread is in the middle of being shutdown.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'd recommend (if you want to get some sort of memory tracking in) to instead write any memory tracking code in native code. It's not as nice as managed code, but as @jkotas points out, there's no reliable way to do this in managed code.