Disabling a restartless add-on doesn't automatically unload JS modules it loaded

NEW
Unassigned

Status

()

Core
XPCOM
7 years ago
7 years ago

People

(Reporter: glandium, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

7 years ago
I've modified the about:startup extension to use a jsm module and register a resource:// url so that some things can be shared between the bootstrap code and the about:startup page itself. It turns out when upgrading the extension between revisions having a jsm module, the old version is always used, because it is kept in startup cache. The only way to make it get the new version is to stop Firefox, remove the startup cache and start Firefox again.

Comment 1

7 years ago
Since we don't implement per-resource validation (and don't want to), it sounds like we should remove the startup cache when an extension is installed or removed. Mossop, does this sound reasonable?
We should already be doing this in the extension manager (by dispatching the observer notification "startupcache-invalidate"), so if that isn't happening then it's a bug.
(Reporter)

Comment 3

7 years ago
Note that currently, the path used in startup cache for that extension is jsloader/resource/aboutstartup/startupdata.jsm.bin.
With bug 620931, it would use the resolved path to construct the startup cache path, e.g. jsloader/home/mh/.mozilla/firefox/g7i9o0v9.4.0/extensions/aboutstartup@glandium.org.xpi/startupdata.jsm.bin, which makes it easier to filter if we want to remove only them.
(Reporter)

Comment 4

7 years ago
(In reply to comment #2)
> We should already be doing this in the extension manager (by dispatching the
> observer notification "startupcache-invalidate"), so if that isn't happening
> then it's a bug.

FWIW, it is a restart-less extension, maybe that's why it doesn't happen?
Oh, I don't think this is anything to do with the startup cache them, the scopes loaded from JS modules are cached in mozJSComponentLoader and at the moment can only be unloaded on shutdown. Any attempt to retrieve a module with the same URL will give you back the same scope. A potential solution is in bug 481603
Though I guess it looks like we don't invalidate the startup cache for restartless extensions either, so looks like we have two problems!

Comment 7

7 years ago
mossop, was your comment 5 resolved by the fix in bug 481603?
If so, can you change the summary of this bug to "...never invalidates restart-less extensions", please?
(In reply to Ben Bucksch (:BenB) from comment #7)
> mossop, was your comment 5 resolved by the fix in bug 481603?
> If so, can you change the summary of this bug to "...never invalidates
> restart-less extensions", please?

Currently the restartless extension still has to manually call Components.utils.unload for all their JS modules, nothing automatic has changed here.

Updated

7 years ago
Summary: Startup cache never invalidates extensions modules → Startup cache and JS modules never invalidated for restart-less extensions
Really we are talking about JS modules, not about the startup cache here. We could in theory know which resource URIs an add-on registered and so automatically unload any JS modules that way, I'm not sure this is something worth spending time on though.
Summary: Startup cache and JS modules never invalidated for restart-less extensions → Disabling a restartless add-on doesn't automatically unload JS modules it loaded

Comment 10

7 years ago
(In reply to Dave Townsend (:Mossop) from comment #6)
> Though I guess it looks like we don't invalidate the startup cache for
> restartless extensions either, so looks like we have two problems!

I guess this should be a different bug, then?

> I'm not sure this is something worth spending time on though.

I think it's better to solve this once in the platform then to let each ext dev run into this (many of them the hard way), and each of them having to work around it with custom code.
(In reply to Ben Bucksch (:BenB) from comment #10)
> (In reply to Dave Townsend (:Mossop) from comment #6)
> > Though I guess it looks like we don't invalidate the startup cache for
> > restartless extensions either, so looks like we have two problems!
> 
> I guess this should be a different bug, then?

This bug was always about JS modules not getting unloaded, it was just mis-diagnosed as being a startupcache problem.

> > I'm not sure this is something worth spending time on though.
> 
> I think it's better to solve this once in the platform then to let each ext
> dev run into this (many of them the hard way), and each of them having to
> work around it with custom code.

If it was straightforward to do then I agree, but it isn't.
You need to log in before you can comment on or make changes to this bug.