Open Bug 1768990 Opened 4 years ago Updated 2 years ago

Enrich XPCOM 'singleton's by shutdown lifecycle management

Categories

(Core :: XPCOM, task)

task

Tracking

()

People

(Reporter: jstutte, Unassigned)

References

(Depends on 3 open bugs)

Details

This bug is meant to collect ideas on how to have a general solution for the shutdown management of singletons.

Currently the 'singleton' property will just remove the code for the Create function. In most cases singletons then provide a custom 'constructor' property used to get the singleton instance.

The constructor is left alone with the decision, if the singleton is still alive and can be used. Furthermore, in case it returns nullptr we will throw an NS_ERROR_OUT_OF_MEMORY which in case of a 'singleton' obfuscates the legitimate possibility that a singleton cannot be used any more (mostly due to shutdown).

We could think of:

  1. Throwing a different error in case singleton is true and the constructor returns nullptr
  2. Having a base class/template, that provides an appropriate singleton factory function and deals with the shutdown lifecycle.

For 2. we could further think of having two singleton lifecycle parameters:

a) ShutdownPhase for an AsyncShutdownBlocker for this instance. This gives the singleton the occasion to do any important things and to cease working (factory will return nullptr afterwards). Appropriate callbacks should be provided.
b) ShutdownPhase for a ClearOnShutdown that will release the associated memory. The possibility of deferring the release wrt the AsyncShutdownBlocker can be used to move the boring part of release into a fast shutdown covered phase (where it could be skipped without danger).

As of nika: We should also consider that "in many cases we've actually handled making the service a singleton seperately, and it can be more performant to just access it from a static or similar rather than going through the component manager"

So 2. should provide also functions for direct singleton access from C++ without going through the component manager with do_GetService.

Depends on: 1796566

I'd actually always intended to eventually extend the singleton property to either forbid instantiating via createInstance or to just always return the cached service instance the way that old singleton constructor macros did. It just wasn't my main priority at the time, and I haven't gotten around to it since.

(In reply to Jens Stutte [:jstutte] from comment #1)

As of nika: We should also consider that "in many cases we've actually handled making the service a singleton seperately, and it can be more performant to just access it from a static or similar rather than going through the component manager"

Note that the static components named accessor should actually be nearly as performant as custom accessors, and removes the need to store a separate reference to the service, which also needs to be cleared separately at shutdown. It should also be more consistent, since if some code uses a custom accessor and some uses the component manager, the component may not always be instantiated the same way (possibly sometimes without first calling module init functions).

For code that doesn't need to be accessed from JS, I would lean towards just using custom accessors and no component registration at all.

(In reply to Jens Stutte [:jstutte] from comment #1)

So 2. should provide also functions for direct singleton access from C++ without going through the component manager with do_GetService.

The named accessors in static component registration do this. They use direct indexed access into the static services table. The only drawback over custom accessors is that they require QueryInterface rather than returning a reasonable interface directly. I considered adding the option to provide an interface or concrete type to return from the getters by default, but decided I'd wait until I had evidence that it made a performance difference (especially given that the new getters are already much more efficient than calling do_GetService).

Depends on: 1760855
You need to log in before you can comment on or make changes to this bug.