asyncStringPool is never garbage collected.

Submitted by Lennart Kats on 12 March 2010 at 12:14

Fixed in r20869 by using a WeakHashMap<String, WeakReference<StrategoString>. A specialized map or set may be more efficient, but this should do the job for now.

Maybe the weak references are too weak?

Running a Stratego program, now and then I’ve got two terms for which fails, but which are actually equal. Confirmed this by stepping through the Java code.

Turns out that both terms contain the same string, but wrapped inside a different StrategoString object. Tracing StrategoString allocations it appears that two objects are allocated by TermFactory.makeString with the same string value (and no annotations). During the second call in TermFactory, there is actually a resultRef but with a referent of null (meaning the string was garbage collected??).

This happens about once in five runs, quite randomly so it seems. Though, it is always the same term for which comparison fails.

@Nathan:

The references are weak but they remain alive as long as a strong reference somewhere else exists. The TermFactory.makeString() method should take care of reuse and maximal sharing of strings to avoid that multiple strings with the same identity exist, which could cause one of the weak references to expire. And even TermFactory.annotateTerm() should use sharing for unannotated strings. So when the referent in TermFactory.makeString() is null, that should indicate that there is no strong reference to that string elsewhere in the program.

Could you post an issue with an example that shows this behavior?

Actually, my key-value pair of type (String, WeakReference) is removed from the string pool at some point. I would figure that means the String has been finalized. But later on this exact String object appears in a term, wrapped by the exact same StrategoString; so they’re definitely not garbage collected.

Maybe this is something to consider: I can see that a few different String object-instances with the same string value exist. They map to the same StrategoString, ie. makeString(String@xxxx = “something”) and makeString(String@yyyy = “something”) return the same StrategoString, while addresses xxxx != yyyy.

@ Nathan:

Strings with different object ids getting the same StrategoString is actually proper behavior: StrategoStrings are maximally shared. There can be one and only one StrategoString for a string value, because that is the only way to guarantee that there are no other StrategoStrings around with the same value, once one StrategoString instance has been garbage collected. Once one StrategoString instance is no longer strongly referenced, then there can be no other StrategoString alive with the same value.

What it comes down to is that we have a (weak String, weak StrategoString) tuple, that should stay alive as long as the following holds:

1) there is a strong reference to the String from the StrategoString

2) there is a strong reference to the StrategoString elsewhere in the program (also required for #1)

These two are always guaranteed as long as the StrategoString remains alive. I found a minor problem in the StrategoString class that would break #1 though, when using multiple term factories together, like Spoofax does. That may have been what was causing your problem.

The minor fix seems to solve my problem, nice catch.

Maybe you should check this out just to be sure: StrategoTerm.internalSetAnnotations can remove the annotations from a StrategoString, creating a un-annotated string without going through the string pool; doesn’t this too break the contract? Also, the assertion in that method seems a bit silly.

The assertion is wrong. I had to look at it for a while until I figured out what I was trying to do there :) Fixed in r21023. Apparently the launch configuration I’m using doesn’t enable assertions for the org.strategoxt.strj package.

internalsetAnnotations() isn’t used for StrategoStrings as they have a MAXIMALLY_SHARED storage type. I added a little extra assertion just to be sure though, you never know what happens in the future…