Bug 1800295 Comment 0 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

I find frequently in my log files:

```
NS_ENSURE_TRUE(inst) failed: StaticComponents.cpp:xxxx
```

and following the line in my build leads me to:

```
    case ModuleID::Anonymous325: {
      RefPtr<mozilla::Viaduct> inst = mozilla::Viaduct::GetSingleton();
      NS_ENSURE_TRUE(inst, NS_ERROR_OUT_OF_MEMORY);
```

which brings me to [`Viaduct::GetSingleton`](https://searchfox.org/mozilla-central/rev/9f2f65d1663b4db2907749e5bbbfe6a81ac43bca/toolkit/components/viaduct/Viaduct.cpp#24) where I read:

```
already_AddRefed<Viaduct> Viaduct::GetSingleton() {
  if (gViaduct) {
    return do_AddRef(gViaduct);
  }

  gViaduct = new Viaduct();
  ClearOnShutdown(&gViaduct);
  return do_AddRef(gViaduct);
}
```

`ClearOnShutdown` has the nice side effect to null out the pointer if we are already in/beyond its phase (by default `ShutdownPhase::XPCOMShutdownFinal`). This is correct in the sense that we should not deliver a new instance such late, but callers seem to not expect this to ever return `nullptr`. I see two things here:

1. Make it more explicit inside `Viaduct::GetSingleton()` for which shutdown phase we expect to cease working. We can use an `AppShutdown::IsInOrBeyond` check for this. Note that it might make sense to not return an instance earlier than we clear it.
2. Check the call sites for proper handling. I can see it [used only by bookmarks](https://searchfox.org/mozilla-central/rev/9f2f65d1663b4db2907749e5bbbfe6a81ac43bca/toolkit/components/places/BookmarkHTMLUtils.sys.mjs#84-86), IIUC. Nika, the tricky part here is that the message is [printed by the generated code](https://searchfox.org/mozilla-central/rev/9f2f65d1663b4db2907749e5bbbfe6a81ac43bca/xpcom/components/gen_static_components.py#449-453), so we might want to tweak/remove that message and leave it to the constructor to complain? We can and should also understand why bookmark's JS runs this late in the first place, though.
I find frequently in my log files:

```
NS_ENSURE_TRUE(inst) failed: StaticComponents.cpp:xxxx
```

and following the line in my build leads me to:

```
    case ModuleID::Anonymous325: {
      RefPtr<mozilla::Viaduct> inst = mozilla::Viaduct::GetSingleton();
      NS_ENSURE_TRUE(inst, NS_ERROR_OUT_OF_MEMORY);
```

which brings me to [`Viaduct::GetSingleton`](https://searchfox.org/mozilla-central/rev/9f2f65d1663b4db2907749e5bbbfe6a81ac43bca/toolkit/components/viaduct/Viaduct.cpp#24) where I read:

```
already_AddRefed<Viaduct> Viaduct::GetSingleton() {
  if (gViaduct) {
    return do_AddRef(gViaduct);
  }

  gViaduct = new Viaduct();
  ClearOnShutdown(&gViaduct);
  return do_AddRef(gViaduct);
}
```

`ClearOnShutdown` has the nice side effect to null out the pointer if we are already in/beyond its phase (by default `ShutdownPhase::XPCOMShutdownFinal`). This is correct in the sense that we should not deliver a new instance such late, but callers seem to not expect this to ever return `nullptr`. I see two things here:

1. Make it more explicit inside `Viaduct::GetSingleton()` for which shutdown phase we expect to cease working. We can use an `AppShutdown::IsInOrBeyond` check for this. Note that it might make sense to not return an instance earlier than we clear it.
2. Check the call sites for proper handling. ~~I can see it [used only by bookmarks](https://searchfox.org/mozilla-central/rev/9f2f65d1663b4db2907749e5bbbfe6a81ac43bca/toolkit/components/places/BookmarkHTMLUtils.sys.mjs#84-86), IIUC.~~ Nika, the tricky part here is that the message is [printed by the generated code](https://searchfox.org/mozilla-central/rev/9f2f65d1663b4db2907749e5bbbfe6a81ac43bca/xpcom/components/gen_static_components.py#449-453), so we might want to tweak/remove that message and leave it to the constructor to complain? We can and should also understand why bookmark's JS runs this late in the first place, though.

Edit: As Nika says below, I mixed up some link here, it seems.

Back to Bug 1800295 Comment 0