Refactor cert store handling. #42
Merged
Conversation
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
![copybara-service[bot]](https://avatars.githubusercontent.com/in/44061?s=60&v=4){kind=small}
copybara-service
bot
force-pushed
the
test_407860877
branch
from
008643f to
8ef3860
Compare
copybara-service
bot
changed the title
copybara-service
bot
force-pushed
the
test_407860877
branch
from
8ef3860 to
a42f104
Compare
With the previous behavior, the stores are closed immediately after any functions using them return. This is potentially problematic, as some functions return handles to the data that's inside the now closed store. It's counter-intuitive to the caller that certtostore may have already allowed Windows to release the store, despite the WinCertStore remaining open. Further, the store was closed with the CHECK_FLAG which allows any other handles associated with the store to leak if the caller doesn't clean them up. The new behavior is to hold the any store handles open inside WinCertStore. These will be freed when the user calls Close() on the WinCertStore. Failure to do so will leak the handle, but this is arguably preferable to trapping the user into vague cases where handles may have been closed prematurely. It also allows the use of the FORCE_FLAG, which will help prevent leaking of associated handles as long as Close() is called. PiperOrigin-RevId: 408645486
copybara-service
bot
force-pushed
the
test_407860877
branch
from
a42f104 to
d1c9527
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Refactor cert store handling.
With the previous behavior, the stores are closed immediately after any functions using them return. This is potentially problematic, as some functions return handles to the data that's inside the now closed store. It's counter-intuitive to the caller that certtostore may have already allowed Windows to release the store, despite the WinCertStore remaining open. Further, the store was closed with the CHECK_FLAG which allows any other handles associated with the store to leak if the caller doesn't clean them up.
The new behavior is to hold the any store handles open inside WinCertStore. These will be freed when the user calls Close() on the WinCertStore. Failure to do so will leak the handle, but this is arguably preferable to trapping the user into vague cases where handles may have been closed prematurely. It also allows the use of the FORCE_FLAG, which will help prevent leaking of associated handles as long as Close() is called.