eclipse-jdt / eclipse-jdt/eclipse.jdt.core

Resource leak analysis when variable is re-assigned

Open
#1,848 0 comments 0 reactions 1 assignee Claimed by @stephan-herrmann View on GitHub
Dominant language
Java
Stars
237
Forks
195
Avg merge
1d 12h
Merged PRs (30d)
47

Description

During work on #1716 I came across more scenarios where flow analysis for resources (closeables) is not yet perfect.

In particular, we struggle when the same variable is assigned in different locations with a method, because now we are interested no only in whether or not the variable holds an unclosed closeable, but also in which instance is bound to the variable at which point - which is specifically interesting for what we call wrapper resources.

The current design allocates a fresh `FakedTrackingVariable` each time a new instance is assigned to a variable, but then we have trouble merging all that information from different FTVs.

My current thinking is to use only one FTV per local variable, but inside the FTV use different `LocalVariableBinding`s from each re-assignment, each having its own flow information. This should help us letting the existing machinery around `FlowInfo` keep track of what is relevant at what point.

Here's an example:
```
import java.io.*;
import org.eclipse.jdt.annotation.*;
class GZIPInputStream extends InputStream {
@Owning InputStream in;
public GZIPInputStream(@Owning InputStream in) throws IOException {
this.in = in;
}
public void close() throws IOException { in.close(); }
public int read() { return -1; }
}
class TarInputStream implements AutoCloseable {
@Owning InputStream in;
public TarInputStream(@Owning InputStream in) throws IOException {
this.in = in;
}
public void close() throws IOException { in.close(); }
}
public class D {

public D(File file) throws IOException {
TarInputStream entryEnumerationStream;
InputStream in = new FileInputStream(file);
try {
in = new GZIPInputStream(in);
} catch(IOException e) {
in.close();
in = new FileInputStream(file);
}
try {
entryEnumerationStream = new TarInputStream(in);
} catch (IOException ex) {
in.close();
throw ex;
}
}
}
```

Here with #1716 we currently report a resource leak againt `in` at `throw ex;` which is "surprising" given the `in.close()` directly before. Debugging tells us that the close() is reported only against one FTV, while another keeps the information of being unclosed.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.