Repository navigation
Free a stack's elements with the stack - #112
Merged
Merged
Conversation
`push!` gives a StackOf its own reference to the element (X509_up_ref, or a copy for extensions, attributes and big numbers), and the stacks OpenSSL returns (`req.extensions`, the CA stack from `unpack`) hand over theirs, but freeing a StackOf, explicitly or by its finalizer, only freed the stack: every element leaked. Pushing a certificate and freeing the stack leaked the certificate (228 MiB over 40,000 rounds; 2.5 MiB now), and each `req.extensions` leaked its extensions. `free` now pops and frees the elements first. `pop!` on an empty stack throws, rather than wrapping the NULL OpenSSL returns. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #112 +/- ##
==========================================
+ Coverage 83.07% 83.12% +0.05%
==========================================
Files 2 2
Lines 1341 1345 +4
==========================================
+ Hits 1114 1118 +4
Misses 227 227 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merged
This file contains hidden or 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
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.
A
StackOf{T}holds a reference to each element:push!takes one:X509_up_reffor certificates, a copy (X509_EXTENSION_dup,X509_ATTRIBUTE_dup,BN_dup) for the rest.req.extensions(X509_REQ_get_extensions) and the CA stack fromunpack(PKCS12_parse).pop!already passes the stack's reference on to the wrapper it returns.But
free(also the finalizer) called onlyOPENSSL_sk_free, which frees the stack and none of its elements. Every element still in a stack when it went leaked: the pushed certificate (its count never reached zero), and the copied extension. Eachreq.extensionscall leaked the request's extensions.Fix
freepops and frees the remaining elements before freeing the stack.pop!on an empty stack throwsArgumentError. Before, it wrapped the NULL thatOPENSSL_sk_popreturns, and the first use of that wrapper crashed.Verification
New testset
StackOwnsElements:CRYPTO_get_ex_new_indexonCRYPTO_EX_INDEX_X509), with GC disabled around the count.main, none (4 == 7fails), and thepop!check fails.Memory growth (maxrss) over N rounds,
main→ this branch:mainreq.extensions(left to the finalizer)Full suite on Julia 1.12
-t 4: passes. The existingStackOftests push, pop,finalizeandfreein every order.Self-review
StackOfthe package makes owns its elements:StackOf{T}()withpush!,get_extensions, andunpack(OpenSSL's documentation: the caller frees*cawithOSSL_STACK_OF_X509_free). No code in the package wraps a borrowed stack such asSSL_get_peer_cert_chain.unpackof a PKCS#12 with no CA certificates leaves*caNULL):OPENSSL_sk_num(NULL)is -1, so nothing is popped.pop!on an empty stack used to return a wrapper around NULL; it now throws, aspop!on an emptyVectordoes.🤖 Generated with Claude Code