bug | The Object Teams Blog

Posts Tagged ‘bug’

More than 3 years ago, on March 18, 2014, Java™ 8 was released, and on the same day Eclipse released support for this new version. I have repeatedly made the point that the Eclipse compiler for Java (ecj) as a second implementation of JLS (after javac) serves the entire Java community as a premier means for quality assurance.

Today, in April 2017, I can report that this effort is still far from complete. Still, JLS, javac and ecj do not define the exact same language. Time to take stock what these differences are about.

My own work on ecj focuses on an aspect that tries hard to remain invisible to casual users: type inference. It’s the heavy machinery behind generic methods, diamond expressions and lambdas, allowing users to omit explicit type information in many places, leaving it to the compiler to figure out the fine print.

To be honest, when we first shipped support for Java 8, I was well expecting lots of bug reports to come in, which would point out corner cases of JLS that we hadn’t implemented correctly. There was one area, that I felt particularly uneasy about: how type inference blends with overload resolution. During the Mars cycle of development Srikanth thankfully performed a major rework and clean up of this exact area.
(I can’t pass the opportunity to report what I learned from this exercise: Overloading is a huge contributor of complexity in the Java language which in (not only) my opinion doesn’t carry its own weight — not a fraction of it).

We are not done with Java 8

The fact that still 2 years after that rework we constantly receive bug reports against Java 8 type inference is unsettling in a way.

To give some numbers to it: during every 6-week milestone we fixed between 1 and 6 bugs in type inference. None of these bugs is solved in a coffee break, some compete for the title “toughest challenge I faced in my career”.

We have a total of 103 bugs explicitly marked as 1.8 inference bugs. Of these

17 were resolved before Java 8 GA
52 have been resolved in the three years since Java 8 GA
34 are still unresolved today.

This will likely keep me busy for at least one more year.

In the early days of Java 8 we could identify two clusters where behavioral differences between javac and ecj could be observed:

wildcard capture
raw types

(I’ll have a note about the latter at the end of this post).

In these areas we could comfort ourselves by pointing to known bugs in javac. We even implemented code to conditionally mimic some of these javac bugs, but frankly, establishing bug compatibility is even more difficult than truthfully implementing a specification.

Meanwhile, in the area of wildcard capture, javac has been significantly improved. Even though some of these fixes appear only in Java 9 early access builds, not in Java 8, we can observe both compilers converging, and given that the major bugs have been fixed, it is getting easier to focus on remaining corner cases. Good.

Java “8.1”

One event almost went under our radar: In February 2015 a revised version of JLS 8 was published. As part of this update, a few sentences have been added on behalf of JDK-8038747. While the spec may be hard to grok by outsiders, the goal can be explained as enabling a compiler to include more type hints from the bodies of lambda expressions that are nested inside a generic method invocation.

In fact, collecting type information from different levels in the AST was a big goal of the type inference rewrite in Java 8, but without the 2015 spec update, compilers weren’t even allowed to look into the body of a lambda, if the lambda does not specify types for its arguments.

m(a -> a.b())

What do we know about b, while types for m and a are still unknown?

Conceptually, this is immensely tricky, because generally speaking, the code in the bodies of such type-elided lambdas can mean just about anything, while the signature of the lambda is not yet known. So we were happy about the spec update, as it promised to resolve a bunch of test cases, where javac accepts programs that ecj – without the update – was not able to resolve.

Ever since, each improved version of ecj created a regression for one or more of our dear users. We debugged no end, read between the lines of JLS, but couldn’t find a solution that would satisfy users in all cases. And they kept complaining that javac had no problems with their programs, even earlier versions of ecj accepted their program, so rejecting it now must be a regression.

“Switching the alliance”

Up-to that point, I saw our main ally in the authors of JLS, Dan Smith and Alex Buckley from Oracle. In particular Dan Smith has been a tremendous help in understanding JLS 8 and analyzing where our reading of it deviated from the authors’ intention. Together we identified not only bugs in my interpretation and implementation of JLS, but also several bugs in javac.

When we iterated bugs relating to JDK-8038747 time and again, this approach was less effective, coming to no conclusion in several cases. I slowly realized, that we were reaching a level of detail that’s actually easier to figure out when working with an implementation, than at the specification level.

This is when I started to seek advice from javac developers. Again, I received very valuable help, now mostly from Maurizio Cimadamore. Previously, my perception was, that JLS is the gold standard, and any deviation or even just liberal interpretation of it is bad. During the discussion with Maurizio I learned, that in some sense javac is actually “better” than JLS, not only in accepting more type-correct programs, but also in terms of better reflecting the intention of the JSR 335 experts.

So I started to deliberately deviate from JLS, too. Instead of “blaming” javac for deviating from JLS, I now “blame” JLS for being incomplete wrt the intended semantics.

To put this effort into proportion, please consider the figure of 103 bugs mentioned above. From these, 17 bugs have a reference to JDK-8038747. Coincidentally, this is the exact same number as those great bug reports prior to Java 8 GA, that gave us the huge boost, enabling us to indeed deliver a high quality implementation right on GA. In other words, this is a huge engineering effort, and we have no idea, how close to done we are. Will we face the next round of regressions on every new release we publish?

If you work from a specification, there is a point where you feel confident that you did all that is required. Knowing that fulfilling the spec is not enough, it’s impossible to say, what is “enough”.

What is “better”?

With wildcard captures and raw types, it was easy to argue, that certain programs must be rejected by a compiler, because they are not type safe and can blow up at runtime in unexpected locations. In the area around JDK-8038747 javac tends to accept more programs than JLS, but here it would be unreasonable to expect javac to change and start rejecting these “good” programs.

Still, calling out a competition of who accepts more “good” programs would be a bad idea, too, because this would completely abandon the goal of equivalence between compilers. After compiling with one compiler, one could never be sure that another compiler would also accept the same program. The term “Java” would loose its precise meaning.

This implies, every attempt to better align ecj with javac, based on knowledge about the implementation and not based on JLS, should be seen as a temporary workaround. To resume its role of leadership, JLS must catch up with any improvements done in the implementation(s).

To comfort the reader, I should say that in all cases discussed here, there’s always a safe fallback: when inference fails to find a solution, it is always possibly to help the compiler by adding some explicit type arguments (or argument types for a lambda). More importantly, such additions, which may be required for one compiler, should never cause problems for another compiler.

Also note, that explicit type arguments are always to be preferred over type casts (which some people tend to use as a workaround): type arguments will help for effective type checking, whereas type casts bypass type checking and can blow up at runtime.

Thanks and Sorry!

I wrote this post in the desire to reach out to our users.

First: Each reproducible bug report is highly valuable; this is what drives JDT code towards higher and higher quality. By accumulating test cases from all these reports we gradually create a test suite that provides the best available safety net.

Second: I am sorry about every regression introduced by any of our fixes, but as this post should explain, we are traveling uncharted territory: some of the corner cases we are currently addressing are not sufficiently covered by JLS. Additionally, type inference is inherently sensitive to the slightest of changes. Predicting, which programs will be affected by a given change in the implementation of type inference is near impossible.

Yet, it’s certainly not a game of “them” vs “us”: JLS, javac, and ecj, we’re all in this together, and only by continuing to talk to each other, eventually we will all speak the same language, when we say “Java 8”. Please bear with us as the saga continues …

PS: Another pet peeve

I am a type system enthusiast, mostly, because I like how type checkers can completely eliminate entire categories of bugs from your programs. I like to give a guarantee that no code that is accepted by the compiler will ever fail at runtime with an exception like attempting to invoke a method that is not present on the receiver, or class cast exceptions in source code that doesn’t mention any class cast. Type inference is the tool that alleviates the verbosity of explicitly typed programs, while at the same time maintaining the same guarantees about type safety.

Unfortunately, there is a class of Java programs for which such guarantees can not be given: if a program uses raw types, the compiler needs to generate lots of checkcast instructions, to make the code acceptable for the JVM. Each of these instructions can cause the program to blow up at runtime in totally unsuspicious locations.

There are situations where javac silently assumes that a raw type List is a subtype of its parameterized form List<String>. This is wrong. Still I cannot just ignore this problem, because lots of “bugs” are reported against ecj, based on the observation that javac and ecj accept different programs, where in many cases the difference concerns the handling of raw types during type inference.

Economically speaking, investigating in the subtleties of how Java 8 compilers handle raw types is a huge waste of efforts. Any one reading this: if you want to do me a favor, and thus help me to focus on the most relevant aspects of compiler development, please clean up your code. If you keep your code private, nobody will suffer except from yourself, but please, before posting a bug report against JDT, if your code example contains raw types, think thrice before submitting the report. Adding proper type arguments will certainly improve the quality of your code. Likely, after that exercise also ecj will be a lot happier with your code and give you correct answers.

Do I need to repeat that raw types were a workaround for migrating towards Java 5? … that raw types were discouraged starting from day 1 of Java 5? If that doesn’t convince you, search on StackOverflow for questions mentioning raw types and type inference, and you will see that by the use of raw types you are basically disabling much of the power of type inference. Let’s please shed the legacy of raw types.

Written by Stephan Herrmann

April 2, 2017 at 21:43

Posted in Eclipse, Uncategorized

Tagged with bug, Java8, type inference

Help the JDT Compiler helping you! – 3: The essence of null annotations

with 2 comments

After my little excursion to detecting resource leaks let me return to our favorite bug: NPE.

Basic support for null annotations was big news at the release of the Eclipse SDK Juno M4.

To fully understand the approach it’s good to sit back and think of what exactly we’re telling the compiler when we add null annotations to a program.

Annotations as filters

One could certainly use the annotations to mean the following:

@NonNull: “I don’t think this variable will ever hold a null, no need to warn me, when I dereference that value.”
@Nullable: “I guess this variable could be null, so please warn me when I forget a null check before dereferencing.”

Interpreted in this way, the annotations would already be helpful, and actually this would rank them in the same category as @SuppressWarnings("null"): no matter what the compiler analyzed up-to this point, please take my word that this thing is / is not dangerous. In both cases we’d ask the compiler to look at certain things and ignore others, because we “know better”.

However, telling the compiler what’s dangerous and what’s not puts the cart before the horse. If I do so, I will be the weakest link in the chain of analysis. I could err, I do err – that’s why I have NPEs in the first place, so I shouldn’t tell the compiler to trust my judgment.

The good news is: when used appropriately null annotations provide a lot more help.

Annotations as an extension of the type system

The essence of static typing

Lets step back and imagine Java wouldn’t have static typing. A variable declaration would be nothing more than a name, introduced -say- using an imaginary keyword var:

   var o;
   o = new Person();
   o.meow();

Right, we could now assign any object, any value, to the variable o and on the other hand we could attempt to invoke any method. Only at runtime will we notice whether the object refered to by o actually has a method meow(). Obviously, this code is unsafe, it could abort saying “Message on understood”. As Java programmers we don’t accept this unsafety, so we use types as a specification:

A typed variable declaration adds a specification with these two sides:

It adds a constraint such that only certain values can legally be assigned to the variable
It establishes a guarantee that certain operations are well-defined wrt the value of the variable.

A statically typed language forces a decision: what values do I want to allow to be bound to the variable? If I declare o as of type Cat the compiler can conclude that “o = new Person();” violates the constraint and cannot be accepted. If, OTOH, I declared o as of type Person the compiler won’t complain at the assignment but typically it will not find a meow() method in a class Person so that line is now illegal. Only if all things match: the declaration, the assignment, and the usage of a variable, only then will the compiler accept the program and certify that this program will not raise “Message not understood”. It’s a trade: constraint for guarantee.
In a statically typed language we are constrained in what we can say, but we gain the guarantee that a certain kind of error will not occur at runtime.

Sounds fair?

More constraints – more guarantees

Standard static typing constrains the program such that values match to the operations we perform with these values, except – there’s a big loop-hole: in traditional object-oriented type systems each class type contains a value that is not suitable for most operations: the value null, which Tony Hoare called his “billion dollar mistake”.

At a closer look, static type checking in Java is founded on a contradiction:

When analyzing an assignment assume that null is a legal value for every object type.
When looking at a method call / a field reference assume that null cannot occur (otherwise no unchecked dereference could be considered as legal).

This is exactly, what null annotations fix: they split each traditional object type into two types:

@NonNull Cat: This is the type that contains only cat values
@Nullable Cat: This is the union of the above type and the null-type which contains only one value: null

You can read the type @Nullable Cat as: either a cat or null.

Null warnings vs. type errors

Those users who try the new feature in the JDT may be surprised to see a whole new kind of error messages. While the original goal is to get alerted about potential NPEs, the compiler may now complain with messages like:

Type mismatch: required ‘@NonNull Cat’ but the provided value can be null

The question may arise, why this is reported as an error, even if no NPE can be directly caused at the statement in question. The answer can be deduced from the following analogy:

void foo(Object o, @Nullable Cat c) {
    Cat aCat = o;                 // "Type mismatch: cannot convert from Object to Cat"
    @NonNull Cat reallyACat  = c; // "Type mismatch: required '@NonNull Cat' but the provided value can be null."
}

(The wording of the second message will be still improved to better reflect different kinds of RHS values).

The analogy shows:

The assignment itself could actually succeed, and even if types don’t match, a language without static typing could actually accept both assignments.
If, however, the assignment were accepted, all subsequent analysis of the use of this variable is useless, because the assumption about the variable’s type may be broken.

Therefor, a first step towards making NPE impossible is to be strict about these rules. Assigning a value to a @NonNull variable without being able to prove that the value is not null is illegal. Just as assigning an Object value to a Cat variable without being able to prove that the value is indeed a cat is illegal.

Interestingly, for the first assignment, Java offers a workaround:

    Cat aCat = (Cat) o;

Using the cast operator has two implications: we tell the compiler that we “know better”, that o is actually a cat (we do believe so) and secondly, as compiler and JVM cannot fully trust our judgment a check operation will be generated that will raise a ClassCastException if our assumption was wrong.

Can we do something similar for @NonNull conversion? Without the help of JSR 308 we cannot use annotations in a cast, but we can use a little helper:

void foo(Object o, @Nullable Cat c) {
    @NonNull Cat reallyACat  = assertNonNull(c);
}
@NonNull  T assertNonNull(T val) {
    if (val == null) throw new NullPointerException("NonNull assertion violated");
    return val;
}

corrected on 2012/03/05

What? We deliberately throw an NPE although the value isn’t even dereferenced? Why that?

The helper mimics exactly what a cast does for normal type conversions: check if the given value conforms to the required type. If not, raise an exception. If the check succeeds re-type the value to the required type.

Here’s an old school alternative:

void foo(@Nullable Cat c) {
    @SuppressWarnings("null") @NonNull Cat reallyACat  = c;
}

(Requires that you enable using @SuppressWarnings for optional errors).

Which approach is better? Throwing an exception as soon as something unexpected happens is far better than silencing the warning and waiting for it to explode sometime later at some other location in the code. The difference is felt during debugging. It’s about blame assignment.
If things blow up at runtime, I want to know which part of the code caused the problem. If I use @SuppressWarnings that part is in stealth mode, and an innocent part of the code will get the blame when it uses the wrong-typed value.

Remember, however, that cast and assertNonNull are not the solution, those are workarounds. Solutions must explicitly perform the check and provide application specific behavior to both outcomes of the check. Just as a cast without an instanceof check is still a land-mine, so is the use of the above helper: NPE can still occur. If you need to dereference a variable that’s not @NonNull you should really ask yourself:

How can it happen that I end up with a null value in this position?
How can the application safely and soundly continue in that situation?

These questions cannot be answered by any tool, these relate to the design of your software.

Help the JDT compiler helping you

This post showed you two things you can and should do to help the compiler helping you:

Add null annotations to resolve the contradiction that’s inherent in Java’s type system: a type can only either contain the value null or not contain the value null. Still Java’s type system opportunistically assumes a little bit of both. With annotations you can resolve the ambiguity and state which of the two possible types you mean.

Second, listen to the new type error messages. They’re fundamental to the analysis. If you disregard (or even disable) these messages there’s no point in letting the analysis apply all its sophisticated machinery. From false assumptions we cannot conclude anything useful.

If you apply these two hints, the compiler will be your friend and report quite some interesting findings. For a project that uses null annotations right from the first line of code written, this advice should be enough. The difficult part is: if you have a large existing code base already, the compiler will have a lot to complain. Think of migrating a fully untyped program to Java. You bet you could use some more help here. Let’s talk about that in future posts.

Written by Stephan Herrmann

February 21, 2012 at 20:39

Posted in Eclipse

Tagged with analysis, bug, JDT, null, warning

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();
  }

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.
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.
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:

If a method allocates a resource, it owns it – initially.
If a method obtains a resource by calling another method, it may potentially be responsible, since we cannot distinguish ownership from lending
If a method passes a resource as an argument to another method (or constructor), it may or may not transfer ownership by this call.
If a method receives a resource as a parameter, it assumes the caller is probably still responsible
If a method passes a resource as its return value back to the caller, it rejects any responsibility
If a resource is ever stored in a field, no single method feels responsible.
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:

Agree that this is a bug
Explain to the compiler why you believe the code is safe (unique assignment to locals, less passing around)
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.

Written by Stephan Herrmann

February 4, 2012 at 21:11

Posted in Eclipse

Tagged with analysis, bug, JDT, warning

Help the JDT Compiler helping you! – 1: Resource Leaks

with 6 comments

During the Juno cycle a lot of work in the JDT has gone into more sophisticated static analysis, and some more is still in the pipe-line. I truly hope that once Juno is shipped this will help all JDT users to find more bugs immediately while still typing. However, early feedback regarding these features shows that users are starting to expect miracles from the analysis 🙂

On the one hand seeing this is flattering, but on the other hand it makes me think we should perhaps explain what exactly the analysis can see and what is beyond its vision. If you take a few minutes learning about the concepts behind the analysis you’ll not only understand its limitations, but more importantly you will learn how to write code that’s better readable – in this case for reading by the compiler. Saying: with only slightly rephrasing your programs you can help the compiler to better understand what’s going on, to the effect that the compiler can answer with much more useful error and warning messages.

Since there’s a lot of analysis in this JDT compiler I will address just one topic per blog post. This post goes to improvements in the detection of resource leaks.

Resource leaks – the basics

Right when everybody believed that Eclipse Indigo RC 4 was ready for the great release, another blocker bug was detected: a simple resource leak basically prevented Eclipse from launching on a typical Linux box if more than 1000 bundles are installed. Coincidentally, at the same time the JDT team was finishing up work on the new try-with-resources statement introduced in Java 7. So I was thinking: shouldn’t the compiler help users to migrate from notoriously brittle handling of resources to the new construct that was designed specifically to facilitate a safe style of working with resources?

What’s a resource?

So, how can the compiler know about resources? Following the try-with-resources concept, any instance of type java.lang.AutoCloseable is a resource. Simple, huh? In order to extend the analysis also to pre Java 7 code, we also consider java.io.Closeable (available since 1.5).

Resource life cycle

The expected life cycle of any resource is : allocate—use—close. Simple again.

From this we conclude the code pattern we have to look for: where does the code allocate a closeable and no call to close() is seen afterwards. Or perhaps a call is seen but not all execution paths will reach that call, etc.

Basic warnings

With Juno M3 we released a first analysis that could now tell you things like:

Resource leak: “input” is never closed
Resource leak: “input” is never closed at this location (if a method exit happens before reaching close())

If the problem occurs only on some execution paths the warnings are softened (saying “potential leak” etc.).

Good, but…

Signal to noise – part 1

It turned out that the analysis was causing significant noise. How come? The concepts are so clear and all code that wouldn’t exhibit the simple allocate—use—close life cycle should indeed by revised, shouldn’t it?

In fact we found several patterns, where these warnings were indeed useless.

Resource-less resources

We learned that not every subtype of Closeable really represents a resource that needs leak prevention. How many times have you invoked close() on a StringWriter, e.g.? Just have a look at its implementation and you’ll see why this isn’t worth the effort. Are there more classes in this category?

Indeed we found a total of 7 classes in java.io that purely operate on Java objects without allocating any resources from the operating system:

StringReader
StringWriter
ByteArrayInputStream
ByteArrayOutputStream
CharArrayReader
CharArrayWriter
StringBufferInputStream

For none of these does it make sense to warn about missing close().

To account for these classes we simply added a white list: if a class is in the list suppress any warnings/errors. This white list consists of exactly those 7 classes listed above. Sub-classes of these classes are not considered.

Wrapper resources

Another group of classes implementing Closeable showed up, that are not strictly resources themselves. Think of BufferedInputStream! Does it need to be closed?

Well? What’s your answer? The correct answer is: it depends. A few examples:

        void wrappers(String content) throws IOException {
                Reader r1, r2, r3, r4;
                r1 = new BufferedReader(new FileReader("someFile"));
                r2 = new BufferedReader(new StringReader(content));
                r3 = new FileReader("somefile");
                r4 = new BufferedReader(r3);
                r3.close();
        }

How many leaks? With same added smartness the compiler will signal only one resource leak: on r1. All others are safe:

r2 is a wrapper for a resource-less closeable: no OS resources are ever allocated here.
r3 is explicitly closed
r4 is just a wrapper around r3 and since that is properly closed, r4 does not hold onto any OS resources at the end.
returning to r1, why is that a leak? It’s a wrapper, too, but now the underlying resource (a FileReader) is not directly closed so it’s the responsibility of the wrapper and can only be triggered by calling close() on the wrapper r1.

EDIT: We are not recommending to close a wrapped resource directly as done with r3, closing the wrapper (r4) is definitely cleaner, and when wrapping a FileOutputStream with a BufferedOutputStream closing the former is actually wrong, because it may lose buffered content that hasn’t been flushed. However, the analysis is strictly focused on resource leaks and for analysing wrappers we narrow that notion to leaks of OS resources. For the given example, reporting a warning against r4 would be pure noise.

Summarizing: wrappers don’t directly hold an OS resource, but delegate to a next closeable. Depending on the nature and state of the nested closeable the wrapper may or may not be responsible for closing. In arbitrary chains of wrappers with a relevant resource at the bottom, closing any closeable in the chain (including the bottom) will suffice to release the single resource. If a wrapper chain is not properly closed the problem will be flagged against the outer-most wrapper, since calling close() at the wrapper will be delegated along all elements of the chain, which is the cleanest way of closing.

Also for wrappers the question arises: how does the compiler know? Again we set up a white list with all wrapper classes we found in the JRE: 20 classes in java.io, 12 in java.util.zip and 5 in other packages (the full lists are in TypeConstants.java, search for “_CLOSEABLES”).

Status and outlook

Yes, a leak can be a stop-ship problem.

Starting with Juno M3 we have basic analysis of resource leaks; starting with Juno M5 the analysis uses the two white lists mentioned above: resource-less closeables and resource wrappers. In real code this significantly reduces the number of false positives, which means: for the remaining warnings the signal-to-noise ratio is significantly better.

M5 will actually bring more improvements in this analysis, but that will be subject of a next post.

Written by Stephan Herrmann

January 26, 2012 at 15:25

Posted in Eclipse, Uncategorized

Tagged with analysis, bug, JDT, warning

A little statistics of the day

Object Teams with Null Annotations

with 5 comments

The recent release of Juno M4 brought an interesting combination: The Object Teams Development Tooling now natively supports annotation-based null analysis for Object Teams (OT/J). How about that? 🙂
NO NPE

The path behind us

Annotation-based null analysis has been added to Eclipse in several stages:

Using OT/J for prototyping: As discussed in this post, OT/J excelled once more in a complex development challenge: it solved the conflict between extremely tight integration and separate development without double maintenance. That part was real fun.
Applying the prototype to numerous platforms: Next I reported that only one binary deployment of the OT/J-based prototype sufficed to upgrade any of 12 different versions of the JDT to support null annotations — looks like a cool product line
Pushing the prototype into the JDT/Core: Next all of the JDT team (Core and UI) invested efforts to make the new feature an integral part of the JDT. Thanks to all for this great collaboration!
Merging the changes into the OTDT: Now, that the new stuff was mixed back into the plain-Java implementation of the JDT, it was no longer applicable to other variants, but the routine merge between JDT/Core HEAD and Object Teams automatically brought it back for us. With the OTDT 2.1 M4, annotation-based null analysis is integral part of the OTDT.

Where we are now

Regarding the JDT, others like Andrey, Deepak and Aysush have beaten me in blogging about the new coolness. It seems the feature even made it to become a top mention of the Eclipse SDK Juno M4. Thanks for spreading the word!

Ah, and thanks to FOSSLC you can now watch my ECE 2011 presentation on this topic.

Two problems of OOP, and their solutions

Now, OT/J with null annotations is indeed an interesting mix, because it solves two inherent problems of object-oriented programming, which couldn’t differ more:

1.: NullPointerException is the most widespread and most embarrassing bug that we produce day after day, again and again. Pushing support for null annotations into the JDT has one major motivation: if you use the JDT but don’t use null annotations you’ll no longer have an excuse. For no good reasons your code will retain these miserable properties:

It will throw those embarrassing NPEs.
It doesn’t tell the reader about fundamental design decisions: which part of the code is responsible for handling which potential problems?

Why is this problem inherent to OOP? The dangerous operator that causes the exception is this:

right, the tiny little dot. And that happens to be the least dispensable operator in OOP.

2.: Objectivity seems to be a central property on any approach that is based just on Objects. While so many other activities in software engineering are based on the insight that complex problems with many stakeholders involved can best be addressed using perspectives and views etc., OOP forces you to abandon all that: an object is an object is an object. Think of a very simple object: a File. Some part of the application will be interested in the content so it can decode the bytes and do s.t. meaningful with it, another part of the application (maybe an underlying framework) will mainly be interested in the path in the filesystem and how it can be protected against concurrent writing, still other parts don’t care about either but only let you send the thing over the net. By representing the “File” as an object, that object must have all properties that are relevant to any part of the application. It must be openable, lockable and sendable and whatnot. This yields
bloated objects and unnecessary, sometimes daunting dependencies. Inside the object all those different use cases it is involved in can not be separated!

With roles objectivity is replaced by a disciplined form of subjectivity: each part of the application will see the object with exactly those properties it needs, mediated by a specific role. New parts can add new properties to existing objects — but not in the unsafe style of dynamic languages, but strictly typed and checked. What does it mean for practical design challenges? E.g, direct support for feature oriented designs – the direct path to painless product lines etc.

Just like the dot, objectivity seems to be hardcoded into OOP. While null annotations make the dot safe(r), the roles and teams of OT/J add a new dimension to OOP where perspectives can be used directly in the implementation. Maybe it does make sense, to have both capabilities in one language 🙂 although one of them cleans up what should have been sorted out many decades ago while the other opens new doors towards the future of sustainable software designs.

The road ahead

The work on null annotations goes on. What we have in M4 is usable and I can only encourage adopters to start using it right now, but we still have an ambitious goal: eventually, the null analysis shall not only find some NPEs in your program, but eventually the absense of null related errors and warnings shall give the developer the guarantee that this piece of code will never throw NPE at runtime.

What’s missing towards that goal:

Fields: we don’t yet support null annotations for fields. This is next on our plan, but one particular issue will require experimentation and feedback: how do we handle the initialization phase of an object, where fields start as being null? More on that soon.
Libraries: we want to support null specifications for libraries that have no null annotations in their source code.
JSR 308: only with JSR 308 will we be able to annotate all occurrences of types, like, e.g., the element type of a collection (think of List)

Please stay tuned as the feature evolves. Feedback including bug reports is very welcome!

Ah, and one more thing in the future: I finally have the opportunity to work out a cool tutorial with a fellow JDT committer: How To Train the JDT Dragon with Ayushman. Hope to see y’all in Reston!

Written by Stephan Herrmann

December 20, 2011 at 22:32

Posted in Eclipse, Object Teams, OTDT

Tagged with analysis, bug, EclipseCon, JDT, modularity, null, presentation, product line, release, views

A use case for Java 7

with 10 comments

Recent stress tests have revealed a few issues in the Eclipse platform where leaking file handles could cause “Too many open files”-style failures.

Until now that kind of issue was notoriously difficult to analyze, and a lot of luck was involved when I spotted one of those bugs by mere code inspection.

However, chances are that this particular kind of bug will soon be a curiosity from the past. How? Java 7 brings a new feature that should be used in exactly this kind of situation: by the new try-with-resources construct leaking of any resources can be safely avoided.

Now, current code certainly doesn’t use try-with-resources, how can we efficiently migrate existing code to using this new feature? As one step I just filed this RFE for a new warning where try-with-resources should probably be used but is not. At this point I can’t promise that we’ll be able to implement the new warning in a useful way, i.e., without too many false positives, but it sounds like a promising task, doesn’t it?

Here’s a mockup of what I’m thinking of:

And this is what the quickfix would create:

Sounds cool?

Written by Stephan Herrmann

June 14, 2011 at 17:11

Posted in Eclipse

Tagged with bug, Java7, quickfix, warning

Why I’m sometimes a bad bug reporter

with 5 comments

OK, I use Eclipse for getting some work done. Eclipse is software so we know it contains bugs. Given that Eclipse is open source software, we all can only expect it to run smoothly if we diligently report back all errors we encounter (and provide steps on how to reproduce etc.). I know all that and I really want to be a good bug reporter because I really want a good experience using Eclipse because I really want to get the work done (and I may even want to sustain the impression that I’ve got everything under control and thus working with Eclipse is a delight).

A task

The other day, I was a very bad bug reporter, and only today I find some time to reason about what happened. This was my task: In preparing the initial contribution for the Object Teams Project I had to rename a bunch of things from org.objectteams to org.eclipse.objectteams. Simply, huh? Back in the days of emacs+bash I might have chosen to just use one big

find . -exec /bin/sed -i -e "s/org.objectteams/org.eclipse.objectteams/g" {} ;

and voila, if fortuna was with me, I might have been done at the return of that single command. But things were actually just a little bit more challenging, like, a few occurrences would have to remain unchanged plus while touching about every single file in our software I was going to also do some clean-up: rename some packages to foo.internal.bar, fixing some copyright headers, etc. Also I preferred to change one plug-in at a time which would mean that all references to plug-ins not yet processed should stay unchanged, too. Plus a few more little deviations from the grand search-and-replace-globally.

OK, since I like to see myself an Eclipse-wizard this was a nice challenge for its refactoring support. Plug-in by plug-in I renamed, I renamed packages with / or without subpackages, and after each step I wanted to see that compiler and PDE agree with all changes and signal that everything is still (or again) consistent, ready to be built, actually. Perhaps things started the get wrong when I estimated the effort as one or two hours. So, after a day or so, I wasn’t perfectly relaxed any more. My fault, should’ve known better about that estimate. BTW, one of the reasons it took so long was simply the size of my workspace in comparison to the power of my computer / hard-disk: every time I performed a rename with updates in non-Java files, I was nervously looking at the screen: “should I sit and wait for the preview page, or should I go to the kitchen, get a chocolate, coffee, just something?“. I did some emailing in parallel, but let’s just keep this: due to those response times
I wasn’t perfectly relaxed any more.

A story of bug reporting

What I was going to tell here is a story of bug reporting, because as a safe bet doing a real-life stress test to an Eclipse component should give you a good chance to discover and report a few bugs that have not yet been reported by others. And indeed, I was successful in discovering some bugs, in various components actually.

I think one of the first things that occurred was that the svn synchronize view would sometimes fail to open the compare editor, more precisely, I had to explicitly close the compare editor before comparing the next file. At first this really **** me off, because the error dialog was popping up in some kind of infinite loop. Fun!#$ Once I’d figure out how to work around this problem it soon became a habit to just close the compare editor before clicking the next. Next, the svn plugin made a refactoring fail, because it was trying to create a directory which the previous refactoring had already created. The most creative bug having to do with subversive was a chain-reaction of first failing to undo a refactoring and than during reporting this error blocking the UI of Eclipse so I could only kill the Eclipse process, insert a new coin and start a new game.

I don’t intend to blame a particular component. For clean-up of license headers I have a little home-grown plugin that I just wanted to quickly install into the running Eclipse instance, so I went for the cool new feature to export/install into the host. Oops, my plugin depends on another plugin that only exists in the workspace but not in the host, install failed for good reasons. I removed the dependency and tried again. Installation still failed for the same reason: the ghost of this removed dependency prevented installation into the host Eclipse. Oh, I should have incremented the version or let a version qualifier do this automatically, of course. Tried again, still failed. Tried something so slightly different I cannot recall, from there on it worked. Can I reproduce the two or three different levels of failure? I didn’t even take the time to think of it. Well I would’ve been disappointed without a bug from p2 in this list 😉 .

PDE did its share by reporting overlapping text edits in plugin.xml and therefore disabling its refactoring participant. What the **** caused those overlapping text edits, and how do I re-enable the refactoring participant to give it one more chance to behave well?

The list could go on if only I could remember. Instead I was happy to finish this 1.5 hours task after 2.7 days, ready to submit our initial code contribution, wow!

Looking back, I / we missed a great opportunity: we could have identified plenty of bugs in various components of Eclipse. With only a few more days of debugging I might have been able to present reproducing steps for all those bugs. And, if triaged and fixed by the corresponding devs, this might have contributed to M6 containing fewer of those bugs that just only occur in real world, never during testing. I failed, I managed only to submit two bug reports, with very little information on how to reproduce.

Lesson learned

Susan McCourt responded to an earlier bug report of mine in a very descriptive comment:

That is one of those things I’ve been meaning to fix forever, never wrote a
bug, and so keep forgetting to fix. And it seems like if I’m actually
[doing what triggers the bug], it’s because something is wrong, and so I again postpone
writing a bug.

Sure, when we hit a bug (or a bug hits us) we are always in some context of doing something challenging. Something that requires our mind to stay in focus. Something we want to get done.
Well, work isn’t perfectly linear, so we know how to react to interrupts. Bugs are such interrupts. Sometimes I like the challenge of isolating a bug etc. Sometimes I’m sufficiently relaxed when the bug occurs so I actually take the challenge. Sometimes the bug is sufficiently good-natured so making a small note and going back to the bug after the actual work is done is a perfect strategy. Some bugs, however, smell like depending on so many factors from your current context that reproduction an hour later seems extremely unlikely.

I think I have a solution to all this: given we don’t want to be distracted from our actual work, given also that some bugs need immediate care or they will escape our attempts to identify. Given some of the worst moments are when we start to isolate a bug and during that task a second bug stops us from working on the first bug etc. The only way to relentlessly follow all those tasks is to enable branching universes in your working environment. The frequent use of the work “task” may give a hint that I should finally start using Mylyn (I have no excuse for not doing so), but I would need a Mylyn that is able to capture full contexts: the complete state of my machine plus the full state of my brain. As a start I’m dreaming of always working in a virtual machine, and whenever something strange happens, I quickly (!!) create a snapshot of the virtual machine. Then I could first isolate (and fix 🙂 ) the bug that just occurred, and then go back to the exact point where I
interrupted my work and act as if nothing had happened. Branching universes with the ability of back porting fixes between branches is what I need. Of course the clock needs to be reset when returning from a bug reporting / fixing branch.

Yeah, that’s why I can’t quite live up to my dreams of good participation in open source development: I haven’t figured out how to enable branching universes for my character. If anybody has a hint on how to do this or any other strategy to not get overwhelmed between work and bug reporting, I’d really appreciate.

And if I don’t smile while writing a bug report, please excuse, I might just be terribly stressed because your bug interrupted my work on isolating another bug that stopped me from doing …

”Always look at the bright side of life…” 🙂

Written by Stephan Herrmann

February 7, 2010 at 18:31

Posted in Eclipse

Tagged with bug, open source, p2, pde, refactoring, svn

Posts Tagged ‘bug’

We are not done with Java 8

Java “8.1”

“Switching the alliance”

What is “better”?

Thanks and Sorry!

PS: Another pet peeve

Annotations as filters

Annotations as an extension of the type system

The essence of static typing

More constraints – more guarantees

Null warnings vs. type errors

Help the JDT compiler helping you

Flow analysis – power and limitation

Resource ownership

The art of being quiet

Resource leaks – the basics

What’s a resource?

Resource life cycle

Basic warnings

Signal to noise – part 1

Resource-less resources

Wrapper resources

Status and outlook

The path behind us

Where we are now

Two problems of OOP, and their solutions

The road ahead

A task

A story of bug reporting

Lesson learned

Recent Posts

Recent Comments

Archives

Categories

Meta