Repository navigation
[java] Track PipedInputStream and PipedOutputStream as resources - #2290
Merged
Merged
Conversation
PipedInputStream and PipedOutputStream are Closeable, but their constructors were not modeled, so Pulse never tracked them. A pipe end that is only closed on the normal path was therefore not reported when a read or write throws (FN_pipedInputStreamNotClosedAfterReadBad and FN_pipedOutputStreamNotClosedAfterWriteBad). Model their constructors as resource allocations that may throw IOException, as connecting to the other end of the pipe can. Reads, writes and close already go through the InputStream, OutputStream and Closeable models.
4 tasks done
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.
Summary
PipedInputStreamandPipedOutputStreamareCloseable, but their constructors were not modeled, so Pulse never tracked them as resources. A pipe end that is only closed on the normal path was not reported when a read or write throws. The existing testsFN_pipedInputStreamNotClosedAfterReadBadandFN_pipedOutputStreamNotClosedAfterWriteBadinResourceLeaks.javashow this.<init>of both classes withResource.allocate ~exn_class_name:"java.io.IOException", since the constructors that connect to the other end of the pipe may throwIOException(same pattern asSocket/ServerSocket).read,write,flushandclosealready go through the existingInputStream,OutputStreamandCloseablemodels, so nothing else is needed.Tests
FN_pipedInputStreamNotClosedAfterReadBadandFN_pipedOutputStreamNotClosedAfterWriteBadare renamed without theFN_prefix and are now reported.pipedInputStreamClosedOkandpipedOutputStreamClosedOkclosed the stream infinallywithout a null check. With the constructor able to throw, Pulse correctly flags that as a null dereference, so I addedif (... != null)beforeclose(), as theFileInputStreamtests already do. They stay clean.Test plan
PULSE_RESOURCE_LEAKfor both, allocated by thePipedInputStream()/PipedOutputStream()constructors; theOkcases stay cleanmake direct_java_pulse_test: the onlyissues.expchanges are those 2 lines. (Locally I still get the unrelated lambda-name differences in InvokeDynamic/TransitiveAccess that also show up on main with my javac; not committed.)ocamlformat0.29.0 reports no changes forPulseModelsJava.ml