The Object Teams Blog

Adding team spirit to your objects.

Help the JDT Compiler helping you! – 2: Resource leaks – continued

with one comment

In my previous post I showed the basics of a new analysis that I originally introduced in the JDT compiler as of 3.8 M3 and improved for M5. This post will give yet more insight into this analysis, which should help you in writing code that the compiler can understand.

Flow analysis – power and limitation

An advantage of implementing leak analysis in the compiler lies in the synergy with the existing flow analysis. We can precisely report whether a resource allocation is definitely followed by a close() or if some execution paths exist, where the close() call is by-passed or an early exit is taken (return or due to an exception). This is pretty cool, because it shows exactly those corner cases in your implementation, that are so easy to miss otherwise.

However, this flow analysis is only precise if each resource is uniquely bound to one local variable. Think of declaring all resource variables as final. If that is possible, our analysis is excellent, if you have multiple assignments to the same variable, if assignments happen only on some path etc, then our analysis can only do a best-effort attempt at keeping track of your resources. As a worst case consider this:

  Reader r = new FileReader(f);
  Reader r2 = null;
  while (goOn()) {
     if(hasMoreContent(r)) {
        readFrom(r);
     } else {
        r.close(); // close is nice, but which resource exactly is being closed??
     }
     if (maybe()) {
        r2 = r;
     }             // at this point: which resource is bound to r2??
     if (hasMoreFiles()) {
        r = new FileReader(getFile()); // wow, we can allocate plenty of resources in a loop
     }
  }
  if (r2 != null)
    r2.close();
 

This code may even be safe, but there’s no way our analysis can keep track of how many resources have been allocated in the loop, and which of these resources will be closed. Which one is the resource flowing into r2 to be closed at the end? We don’t know. So if you want the compiler to help you, pretty please, avoid writing this kind of code 🙂

So what rules should you follow to get on terms with the compiler? To understand the mentioned limitation it helps to realize that our analysis is mostly connected to local variables, keeping some status bits for each of them. However, when analyzing variables the analysis has no notion of values, i.e., in the example the compiler can only see one variable r where at runtime an arbitrary number of Reader instances will be allocated, bound and dropped again.

Still, there are three special situations which the analysis can detect:

  Reader r = new FileReader("someFile");
  r = new FileReader("otherFile");
  r = new BufferedReader(r);
  Reader r2 = getReader();
  if (r2 != null) {
     r2.close();
  }
 
  1. In line 2 we’re leaking the instance from line 1, because after the assignment we no longer have a reference to the first reader and thus we cannot close it.
  2. However, line 3 is safe, because the same reader that is being dropped from r is first wrapped into a new BufferedReader and indirectly via that wrapper it is still reachable.
  3. Finally at the end of the example snippet, the analysis can see that r2 is either null or closed, so all is safe.

You see the compiler understands actually a lot of the semantics.

My fundamental advice is:

If the compiler warns about leaking resources and if you think the warning is unnecessary, try to better explain why you think so, first of all by using exactly one local variable per resource.

Resource ownership

Still not every method lacking a close() call signifies a resource leak. For an exact and definite analysis we would need one more piece of information: who owns any given resource?

Consider a group of methods happily passing around some resources among themselves. For them the same happens as for groups of people: diffusion of responsibility:

Well, no, I really thought that you were going to close this thing?!!?”.

If we had a notion of ownership we’d simple require the unique owner of each resource to eventually close that resource. However, such advanced concepts, while thoroughly explored in academia, are lacking from Java. To mitigate this problem, I made the following approximations of an ownership model:

  1. If a method allocates a resource, it owns it – initially.
  2. If a method obtains a resource by calling another method, it may potentially be responsible, since we cannot distinguish ownership from lending
  3. If a method passes a resource as an argument to another method (or constructor), it may or may not transfer ownership by this call.
  4. If a method receives a resource as a parameter, it assumes the caller is probably still responsible
  5. If a method passes a resource as its return value back to the caller, it rejects any responsibility
  6. If a resource is ever stored in a field, no single method feels responsible.
  7. If a resource is wrapped in an array we can no longer track the resource, but maybe the current method is still responsible?

In this list, green means: the compiler is encouraged to report anything fishy as a bug. Blue means, we still do the reporting, but weaken the message by saying “Potential resource leak”. Red means, the compiler is told to shut up because this code could only be checked by whole system analysis (which is not feasible for an incremental compiler).

The advice that follows from this is straight-forward:

Keep the responsibility for any resource local.
Do not pass it around and don’t store it in fields.
Do not talk to any strangers about your valuable resources!

In this regard, unclean code will actually cancel the leak analysis. If ownership of a resource is unclear, the compiler will just be quiet. So, do you think we should add a warning to signal whenever this happens? Notably, a warning when a resource is stored in a field?

The art of being quiet

Contrary to naive thinking, the art of good static analysis is not in reporting many issues. The art is in making yourself heard. If the compiler just rattles on with lots of uninteresting findings, no one will listen, no one will have the capacity to listen to all that.

A significant part of the work on resource leak analysis has gone into making the compiler quieter. And of course this is not just a matter of turning down the volume, but a matter of much smarter judgment of what the user might be interested in hearing.

By way of two recently resolved bugs (358903 and 368546) we managed to reduce the number of resource leak warnings reported against the sources of the Eclipse SDK from almost 100 down to 8. Calling this a great success may sound strange at first, but that it is.

At the level we reached now, I can confidently encourage everybody to enable this analysis (my recommendation: resource leaks = error, potential resource leaks = warning). The “Resource leak” problems indeed deserve a closer look, and also the potential ones could give valuable hints.

For each issue reported by the compiler you have three options:

  1. Agree that this is a bug
  2. Explain to the compiler why you believe the code is safe (unique assignment to locals, less passing around)
  3. Add @SuppressWarnings("resource") to tell the compiler that you know what you are doing.

But remember the nature of responsibility: if you say you don’t want to here any criticism you’d better be really sure. If you say you take the responsibility the compiler will be the humble servant who quietly forgets all worries.

Finally, if you are in the lucky position to use Java 7 for your projects, do the final step: enable “Resource not managed with try-with-resource” analysis. This was actually the start of this long journey: to let the compiler give hints where this new syntax would help to make your code safer and to make better visible why it is safe – with respect to resource leaks.

Final note: one of the bugs mentioned above was only resolved today. So with M5 you will still see some avoidable false positives. The next build from now should be better 🙂

I’ll be back, soon, with more on our favorite exception: NPE.

Advertisements

Written by Stephan Herrmann

February 4, 2012 at 21:11

Posted in Eclipse

Tagged with , , ,

One Response

Subscribe to comments with RSS.

  1. […] my little excursion to detecting resource leaks let me return to our favorite bug: […]


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: