Double brace initialization of static members

Rule java:S3599 says not to use so-called double brace initialization (initializer of an anonymous class) because of the possibility of leaks (since the anonymous class object includes a pointer to the parent object, preventing that object from being garbage collected).

There’s a lot of discussion about that online. Some people claim that memory leaks aren’t an issue if the anonymous class is assigned to a static member, which would make DBI at worst an unfamiliar (for many people) construct.

Is this true? If so, could we have the rule make an exception when the field is static? For instance:

Noncompliant Code Example (from SQ):

Map source = new HashMap(){{ // Noncompliant
    put("firstName", "John");
    put("lastName", "Smith");
}};

Compliant:

public static final Map source = ....

Perhaps have a switch to select this option (because some people may still consider DBI undesirable even for static members)?

I don’t know if I would classify this as a False Positive. Like I said, some people might still consider this feature something that makes code harder to read, so is at least a code smell. For instance, this post (Don’t be “Clever”: The Double Curly Braces Anti Pattern – Java, SQL and jOOQ. – scroll down to “So, please, never use this anti pattern”) argues DBI shouldn’t be used even for static methods.

So my points would be:

  1. Is the claim on the blog above true? That is, using DBI for a static method eliminates the risk of memory leaks?

  2. Even if #1 is true, some people argue that DBI should be avoided. So SQ users should have the choice of whether or not to flag DBI in places where leaks are not a concern. Which could be done by having a user-settable switch on the rule, so one could activate the rule while allowing DBI on static methods.

Hey @MisterPi

While more a suggestion that an FP/FN, this is probably the most sensible place to land so that it gets the attention of our language experts.

That “antipattern” looks like a jdk optimization problem, but, the article is from 2015! Do you know if that memory leak stills on modern versions?

Anyway is the kind of thing that magically fixes with the time.

IDK, but the folks at Sonar think it’s still a problem, since the rule has a “leak” tag.

It sounds to me like the problem cited (the anonymous object has a pointer to the parent object) is inherent in the language and not just a JDK issue. But I’m not a Java guru so I was hoping people who ARE would have more insight.

I cited the article because it makes the claim that using it for static members is OK since the “leak” problem doesn’t happen – and then goes on to say you STILL shouldn’t do it, because it’s hard to understand, and the next dev might come along and delete the ‘static’ keyword.

Hello @MisterPi,

Regardless of potential memory leaks from DBI, S3599 is really about improving the readability of your code. To solve the issue, there are 2 possible avenues depending on the Java version you are using.

  1. You are using Java 8 and the suggestions in the rule description are appropriate.

  2. You are using java 9+ and you should really be using convenience factory methods

static final Map<String, String> immutable =  Map.of(
  "firstName", "John",
  "lastName", "Smith"
);

And even if you are initializing a mutable static field, you can still get around it.

static final Map<String, String> mutable = new HashMap(
  Map.of(
    "firstName", "John",
    "lastName", "Smith"
  )
);

It is a tad more verbose but better than what you had before the DBI.

And finally, whether all/some JVM implementations can prevent memory leaks from DBI is not necessarily useful information because we cannot tell where your code will be running in the future.

Hope this helps,

Dorian

But if it’s just a readability issue and not a genuine leak then that would make it a smell rather than a bug. I talked about this with another rule (S2111 should check for integer equivalence) where I suggested having the cases that are merely “smelly” be marked as smells rather than bugs. (In fact, that’s what we did in our code, with web_api and some PowerShell scripting, where we “demoted” over 80% of our java:S2111 “bugs” to “smells” since the QA folks are mainly focused on bugs.)

But my original question still stands: does the possibility of a leak go away if I restrict DBI to static fields? In our code, SQ flags 3 DBI instances, but all are for collections which are declared public static final, each in a class which consists only of constants. (I’m asking from a language perspective, rather than a JVM issue like Andoni did. Oh, BTW, welcome to our community Andoni! :slight_smile: )

1 Like