Repository navigation
Return an error instead of crashing when a Writer is used after Close - #175
Open
vignesh2027 wants to merge 1 commit into
Open
vignesh2027 wants to merge 1 commit into
vignesh2027 wants to merge 1 commit into
Conversation
Close frees the C compression stream and sets ctx to nil, but Write, Flush and SetNbWorkers still passed that nil ctx to libzstd. Calling any of them after Close (or after a Close that failed on the underlying writer) crashed the process with SIGSEGV in cgo instead of returning an error. They now return errWriterClosed, the same way reader.Read returns errReaderClosed.
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.
Calling Write, Flush or SetNbWorkers on a Writer after Close crashes the whole process instead of returning an error.
Since #163, Close frees the C compression stream and sets
w.ctxto nil, which fixed the leak and made double Close safe. The other Writer methods do not check for that, so they pass a nilZSTD_CCtxto libzstd and the program dies with a SIGSEGV inside cgo. Because it happens in C code,recovercannot catch it.The same thing happens after a Close that fails because the underlying writer returned an error, since that path also nils
ctxwithout settingfirstError.This change makes Write, Flush and SetNbWorkers return a new
errWriterClosedwhenctxis nil, the same wayreader.Readalready returnserrReaderClosedafter Close. An earlierfirstErroris still returned first, and a second Close still returns nil, so the existing tests from #163 keep passing.I added two tests next to the #163 ones. On main both crash the test binary with SIGSEGV in
ZSTD_compressStream2_wrapper, and with the fix they pass. I rango testwith the CI payload,go test -race, and the suite with-tags external_libzstdagainst a system libzstd, all green.