1609444 - GMP thread can write to disk (to clear storage or remove plugins) at xpcom-shutdown-threads time

Status: NEW

(Core :: Audio/Video: GMP, task, P3)

Description

[:Gijs (he/him)]

We're auditing whether we can shut down sooner, to make shutdown shorter. As part of this, we're evaluating what work happens in observers of xpcom-shutdown-threads. This includes GMPServiceParent.cpp

For this to work, we need to be sure we do not in any way rely on writes that happen on this thread, that could therefore be missed / not happen if shutdown goes ahead without "cleanly" shutting down this thread.

In the case of the GMPServiceParent, it seems that we create the thread and offer an abstract mechanism to dispatch tasks to this thread via GMPDispatch.

This includes the following tasks that can write to disk:

• Incompatible version of GMP storage causing storage clearing - this first resets the pref and then clears storage off disk. This would probably need to block shutdown individually or ensure that we do not update the pref until the storage has been cleared (unclear to me if that is likely to cause other issues.)
• the same, but in response to browser:purge-session-history, either to clear everything or just entries after a certain time. I haven't investigated how this interacts with "clear history on shutdown" from our sanitizer code, but I fear the answer might be "messily".
• Similarly, we can delete things when asked to forget a specific site.
• Removing various things from disk - this utility runnable is called by RemovePluginDirectory and RemoveAndDeletePluginDirectory . Only the latter actually deletes from disk, but if the plugin is in use by a content process it will wait for that to no longer be the case, and then delete it.

One solution here would be for the deletions to have their own shutdown blocker, or to move the shutdown response earlier (e.g. profile-before-change - I suspect this code needs the profile anyway?). I'm also a little puzzled because this comment in XPCOMInit suggests media code will shut down before xpcom-shutdown-threads, so perhaps there's something else that already guarantees all these deletions have finished before we shut down?

Comment 1

[Alex Thayer [:alexical] (she/her)]

FWIW it looks like that comment in XPCOMInit used to be before xpcom-shutdown-threads.

Comment 2

[Alex Thayer [:alexical] (she/her)]

Taking a second look at this - it looks like this already ensures that everything is cleared out of mGMPThread's queue during the profile-change-teardown event. In UnloadPlugins it adds an event to the back of its queue which sets the mWaitingForPluginsSyncShutdown flag to false, which the main thread spins the event loop waiting for. I think this might not be airtight, but it looks like it's trying to not do anything past that point, so if we give it a little bit of extra sealing we probably won't break anything.