Avoid Java double brace initialization

Avoid Java double brace initialization

TL;DR

Avoid doing this:

new HashMap<String, String>() {{
  put("key", value);
}};

Leak Trace

I was recently looking at the following leak trace from LeakCanary:

┬───
│ GC Root: Global variable in native code
│
├─ com.bugsnag.android.AnrPlugin instance
│    Leaking: UNKNOWN
│    ↓ AnrPlugin.client
│                ~~~~~~
├─ com.bugsnag.android.Client instance
│    Leaking: UNKNOWN
│    ↓ Client.breadcrumbState
│             ~~~~~~~~~~~~~~~
├─ com.bugsnag.android.BreadcrumbState instance
│    Leaking: UNKNOWN
│    ↓ BreadcrumbState.store
│                      ~~~~~
├─ com.bugsnag.android.Breadcrumb[] array
│    Leaking: UNKNOWN
│    ↓ Breadcrumb[494]
│                ~~~~~
├─ com.bugsnag.android.Breadcrumb instance
│    Leaking: UNKNOWN
│    ↓ Breadcrumb.impl
│                 ~~~~
├─ com.bugsnag.android.BreadcrumbInternal instance
│    Leaking: UNKNOWN
│    ↓ BreadcrumbInternal.metadata
│                         ~~~~~~~~
├─ com.example.MainActivity$1 instance
│    Leaking: UNKNOWN
│    Anonymous subclass of java.util.HashMap
│    ↓ MainActivity$1.this$0
│                     ~~~~~~
╰→ com.example.MainActivity instance
​     Leaking: YES (​Activity#mDestroyed is true)

When opening a leak trace, my first step is to look at the object at the bottom to understand what its lifecycle is, as that'll help me understand if other objects in the leak trace are expected to have the same lifecycle or not.

At the bottom, we see:

╰→ com.example.MainActivity instance
​     Leaking: YES (​Activity#mDestroyed is true)

The activity is destroyed and should have been garbage collected, but it's held in memory.

So, at that point, I start looking for known types in the leak trace and try to figure out if they belong to the same destroyed scope (=> they're leaking) or to a higher scope (=> they're not leaking).

At the top, we see:

├─ com.bugsnag.android.Client instance
│    Leaking: UNKNOWN

Our BugSnag client is a singleton that we use for crash reporting purposes, we create a single instance per app, so it's not leaking.

├─ com.bugsnag.android.Client instance
│    Leaking: NO

So now we can update the leak trace to focus only on the section from the last Leaking: NO to the first Leaking: YES:

…
├─ com.bugsnag.android.Client instance
│    Leaking: NO
│    ↓ Client.breadcrumbState
│             ~~~~~~~~~~~~~~~
├─ com.bugsnag.android.BreadcrumbState instance
│    Leaking: UNKNOWN
│    ↓ BreadcrumbState.store
│                      ~~~~~
├─ com.bugsnag.android.Breadcrumb[] array
│    Leaking: UNKNOWN
│    ↓ Breadcrumb[494]
│                ~~~~~
├─ com.bugsnag.android.Breadcrumb instance
│    Leaking: UNKNOWN
│    ↓ Breadcrumb.impl
│                 ~~~~
├─ com.bugsnag.android.BreadcrumbInternal instance
│    Leaking: UNKNOWN
│    ↓ BreadcrumbInternal.metadata
│                         ~~~~~~~~
├─ com.example.MainActivity$1 instance
│    Leaking: UNKNOWN
│    Anonymous subclass of java.util.HashMap
│    ↓ MainActivity$1.this$0
│                     ~~~~~~
╰→ com.example.MainActivity instance
​     Leaking: YES (​Activity#mDestroyed is true)

The BugSnag client keeps a ring buffer of breadcrumbs. Those are meant to stay in memory, they're not leaking. So let's update again:

├─ com.bugsnag.android.BreadcrumbInternal instance
│    Leaking: NO

Once again, we update the leak trace to focus only on the section from the last Leaking: NO to the first Leaking: YES:

…
├─ com.bugsnag.android.BreadcrumbInternal instance
│    Leaking: NO
│    ↓ BreadcrumbInternal.metadata
│                         ~~~~~~~~
├─ com.example.MainActivity$1 instance
│    Leaking: UNKNOWN
│    Anonymous subclass of java.util.HashMap
│    ↓ MainActivity$1.this$0
│                     ~~~~~~
╰→ com.example.MainActivity instance
​     Leaking: YES (​Activity#mDestroyed is true)
  • BreadcrumbInternal.metadata: the leak trace goes through the metadata field of the breadcrumb implementation.
  • MainActivity$1 instance Anonymous subclass of java.util.HashMap: MainActivity$1 is an anonymous subclass of HashMap, defined in MainActivity. It's the first anonymous class defined from the top of MainActivity.java (because $1).
  • this$0: every anonymous class has an implicit field reference to the outer class in which it is defined, and that field is always named this$0.

Translated to English: one of the breadcrumbs logged to BugSnag has a metadata map that is an anonymous subclass of HashMap that holds a reference to an outer class, the destroyed activity.

Let's look at where we log breadcrumbs in MainActivity:

void logSavingTicket(String ticketId) {
  Map<String, Object> metadata = new HashMap<String, Object>() {{
    put("ticketId", ticketId);
  }};
  bugsnagClient.leaveBreadcrumb("Saving Ticket", metadata, LOG);
}

This code is leveraging a fun Java pattern known as double brace initialization. It allows you to create a HashMap and initialize it at the same time by adding code to the constructor of an anonymous subclass of HashMap.

new HashMap<String, Object>() {{
  put("ticketId", ticketId);
}};

Java anonymous classes always have implicit references to their outer class. So this code:

void logSavingTicket(String ticketId) {
  Map<String, Object> metadata = new HashMap<String, Object>() {{
    put("ticketId", ticketId);
  }};
  bugsnagClient.leaveBreadcrumb("Saving Ticket", metadata, LOG);
}

is actually compiled as:

class MainActivity$1 extends HashMap<String, Object> {
  private final MainActivity this$1;

  MainActivity$1(MainActivity this$1, String ticketId) {
     this.this$1 = this$1;
     put("ticketId", ticketId);
  }
}

void logSavingTicket(String ticketId) {
  Map<String, Object> metadata = new MainActivity$1(this, ticketId);
  bugsnagClient.leaveBreadcrumb("Saving Ticket", metadata, LOG);
}

As a result, the breadcrumb is holding on to the destroyed activity instance.

Conclusion

Avoid using Java double brace initialization, it's cute but creates additional classes for no good reason and risks introducing leaks. Instead, you can do things the boring and safe way:

Map<String, Object> metadata = new HashMap<>();
metadata.put("ticketId", ticketId);
bugsnagClient.leaveBreadcrumb("Saving Ticket", metadata, LOG);

Or leverage Collections.singletonMap() which make this nicer:

Map<String, Object> metadata = singletonMap("ticketId", ticketId);
bugsnagClient.leaveBreadcrumb("Saving Ticket", metadata, LOG);

Or just convert the file to Kotlin.

Header image generated by DALL-E, prompt: "A coffee mug wearing orthodontic braces, digital art".