wine/gecko
0
0
Fork 0
mirror of https://gitlab.winehq.org/wine/wine-gecko.git synced 2024-09-13 09:24:08 -07:00
Code Issues Packages Projects Releases Wiki Activity

Bug 1082001 - Cleanup settings lock from parent itself. r=bent

Browse Source 
From bug 1065128 SettingsManager has been changed to listen the
dom-window-destroyed event for its cleanup. However, when running Gaia
in Mulet, a race condition is exposed. For B2G, when loading a page,
about:blank is first used. This means that window destroyed events will
be triggered. However, from the dom-window-destroyed event we cannot
distinguish whether this is about:blank or a legit application being
closed. SettingsManager gets initialized (i.e., init() called) when the
application makes use of navigator.mozSettings. So the chain of event is
that we have a SettingsManager living because System app did some
request. At this time, about:blank is being unloaded and triggers a
dom-window-destroyed event. This makes SettingsManager doing its
cleanup, especially freeing the window reference. Then in the meantime,
we have the navigator.mozSettings use that is progressing. At some
point, SettingsManager has no more window to send messages to, and Gaia
is not able to even start.

SettingsRequestManager lives on the parent process and SettingsManager
lives on the child side. Part of the cleanup performed by
SettingsManager was to ensure pending locks on the parent process would
be forced to finalize to make sure those are being properly committed.
We move this cleanup to SettingsRequestManager and we augment the lock
informations with the proper inner window id. This way we can track
which lock is attached to which inner window when the lock gets created.
And thus we can listen on inner-window-destroyed from
SettingsRequestManager to be able to force finalize on any pending lock.
Impacted code path are those were we are not running out of process.
When we are running out of process, SettingsRequestManager already
listens on the child-process-shutdown event to perform the lock
finalization.
This commit is contained in:
Alexandre Lissy 2014-10-29 02:36:00 -04:00
parent bfda9ecd0e
commit 1c23ba77af
2 changed files with 77 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
dom/settings/SettingsManager.js
View File

@ -55,7 +55,12 @@ function SettingsLock(aSettingsManager) {
"Settings:Clear:OK", "Settings:Clear:KO",
"Settings:Set:OK", "Settings:Set:KO",
"Settings:Finalize:OK", "Settings:Finalize:KO"]);
this.sendMessage("Settings:CreateLock", {lockID: this._id, isServiceLock: false});
let createLockPayload = {
lockID: this._id,
isServiceLock: false,
windowID: this._settingsManager.innerWindowID
};
this.sendMessage("Settings:CreateLock", createLockPayload);
Services.tm.currentThread.dispatch(this._closeHelper.bind(this), Ci.nsIThread.DISPATCH_NORMAL);
// We only want to file closeHelper once per set of receiveMessage calls.
@ -255,7 +260,6 @@ function SettingsManager() {
this._callbacks = null;
this._isRegistered = false;
this._locks = [];
this._principal = null;
}
SettingsManager.prototype = {
@ -376,17 +380,18 @@ SettingsManager.prototype = {
if (DEBUG) debug("SettingsManager init");
mrm.registerStrongReporter(this);
cpmm.addMessageListener("Settings:Change:Return:OK", this);
this._window = aWindow;
this._principal = this._window.document.nodePrincipal;
Services.obs.addObserver(this, "dom-window-destroyed", false);
Services.obs.addObserver(this, "inner-window-destroyed", false);
let util = aWindow.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils);
this.innerWindowID = util.currentInnerWindowID;
this._window = aWindow;
},
observe: function(aSubject, aTopic, aData) {
if (aTopic == "dom-window-destroyed") {
let window = aSubject.QueryInterface(Ci.nsIDOMWindow);
if (window == this._window) {
if (DEBUG) debug("Topic: " + aTopic);
if (aTopic === "inner-window-destroyed") {
let wId = aSubject.QueryInterface(Ci.nsISupportsPRUint64).data;
if (wId === this.innerWindowID) {
if (DEBUG) debug("Received: inner-window-destroyed for valid innerWindowID=" + wId + ", cleanup.");
this.cleanup();
}
}
@ -417,21 +422,11 @@ SettingsManager.prototype = {
},
cleanup: function() {
Services.obs.removeObserver(this, "dom-window-destroyed");
Services.obs.removeObserver(this, "inner-window-destroyed");
cpmm.removeMessageListener("Settings:Change:Return:OK", this);
mrm.unregisterStrongReporter(this);
// At this point, the window is dying, so there's nothing left
// that we could do with our lock. Go ahead and run finalize on
// it to make sure changes are commited.
for (let i = 0; i < this._locks.length; ++i) {
if (DEBUG) debug("Lock alive at destroy, finalizing: " + this._locks[i]);
cpmm.sendAsyncMessage("Settings:Finalize",
{lockID: this._locks[i]},
undefined,
this._principal);
}
this.innerWindowID = null;
this._window = null;
this._innerWindowID = null;
},
classID: Components.ID("{c40b1c70-00fb-11e2-a21f-0800200c9a66}"),

93
dom/settings/SettingsRequestManager.jsm
View File

@ -18,6 +18,7 @@ Cu.import("resource://gre/modules/Services.jsm");
Cu.import("resource://gre/modules/PermissionsTable.jsm");
const kXpcomShutdownObserverTopic = "xpcom-shutdown";
const kInnerWindowDestroyed = "inner-window-destroyed";
const kMozSettingsChangedObserverTopic = "mozsettings-changed";
const kSettingsReadSuffix = "-read";
const kSettingsWriteSuffix = "-write";
@ -71,12 +72,14 @@ let SettingsPermissions = {
};
function SettingsLockInfo(aDB, aMsgMgr, aLockID, aIsServiceLock) {
function SettingsLockInfo(aDB, aMsgMgr, aLockID, aIsServiceLock, aWindowID) {
return {
// ID Shared with the object on the child side
lockID: aLockID,
// Is this a content lock or a settings service lock?
isServiceLock: aIsServiceLock,
// Which inner window ID
windowID: aWindowID,
// Tasks to be run once the lock is at the head of the queue
tasks: [],
// This is set to true once a transaction is ready to run, but is not at the
@ -171,6 +174,7 @@ let SettingsRequestManager = {
ppmm.addMessageListener(msgName, this);
}).bind(this));
Services.obs.addObserver(this, kXpcomShutdownObserverTopic, false);
Services.obs.addObserver(this, kInnerWindowDestroyed, false);
},
_serializePreservingBinaries: function _serializePreservingBinaries(aObject) {
@ -648,7 +652,7 @@ let SettingsRequestManager = {
},
observe: function(aSubject, aTopic, aData) {
if (DEBUG) debug("observe");
if (DEBUG) debug("observe: " + aTopic);
switch (aTopic) {
case kXpcomShutdownObserverTopic:
this.messages.forEach((function(msgName) {
@ -657,6 +661,12 @@ let SettingsRequestManager = {
Services.obs.removeObserver(this, kXpcomShutdownObserverTopic);
ppmm = null;
break;
case kInnerWindowDestroyed:
let wId = aSubject.QueryInterface(Ci.nsISupportsPRUint64).data;
this.forceFinalizeChildLocksNonOOP(wId);
break;
default:
if (DEBUG) debug("Wrong observer topic: " + aTopic);
break;
@ -745,42 +755,59 @@ let SettingsRequestManager = {
}
},
removeMessageManager: function(aMsgMgr, aPrincipal) {
if (DEBUG) debug("Removing message manager");
hasLockFinalizeTask: function(lock) {
// Go in reverse order because finalize should be the last one
for (let task_index = lock.tasks.length; task_index >= 0; task_index--) {
if (lock.tasks[task_index]
&& lock.tasks[task_index].operation === "finalize") {
return true;
}
}
return false;
},
enqueueForceFinalize: function(lock, principal) {
if (!this.hasLockFinalizeTask(lock)) {
if (DEBUG) debug("Alive lock has pending tasks: " + lock.lockID);
this.queueTask("finalize", {lockID: lock.lockID}, principal).then(
function() {
if (DEBUG) debug("Alive lock " + lockId + " succeeded to force-finalize");
},
function(error) {
if (DEBUG) debug("Alive lock " + lockId + " failed to force-finalize due to error: " + error);
}
);
}
},
forceFinalizeChildLocksNonOOP: function(windowId) {
if (DEBUG) debug("Forcing finalize on child locks, non OOP");
for (let lockId of Object.keys(this.lockInfo)) {
let lock = this.lockInfo[lockId];
if (lock.windowID === windowId) {
let principal = this.mmPrincipals.get(lock._mm);
this.enqueueForceFinalize(lock, principal);
}
}
},
forceFinalizeChildLocksOOP: function(aMsgMgr, aPrincipal) {
if (DEBUG) debug("Forcing finalize on child locks, OOP");
let msgMgrPrincipal = this.mmPrincipals.get(aMsgMgr);
this.removeObserver(aMsgMgr);
let lockIDs = Object.keys(this.lockInfo);
for (let i in lockIDs) {
let lockId = lockIDs[i];
for (let lockId of Object.keys(this.lockInfo)) {
let lock = this.lockInfo[lockId];
if (lock._mm === aMsgMgr && msgMgrPrincipal === aPrincipal) {
let is_finalizing = false;
let task_index;
// Go in reverse order because finalize should be the last one
for (task_index = lock.tasks.length; task_index >= 0; task_index--) {
if (lock.tasks[task_index]
&& lock.tasks[task_index].operation === "finalize") {
is_finalizing = true;
break;
}
}
if (!is_finalizing) {
this.queueTask("finalize", {lockID: lockId}, aPrincipal).then(
function() {
if (DEBUG) debug("Lock " + lockId + " with dead message manager finalized");
},
function(error) {
if (DEBUG) debug("Lock " + lockId + " with dead message manager NOT FINALIZED due to error: " + error);
}
);
}
this.enqueueForceFinalize(lock, aPrincipal);
}
}
},
receiveMessage: function(aMessage) {
if (DEBUG) debug("receiveMessage " + aMessage.name);
if (DEBUG) debug("receiveMessage " + aMessage.name + ": " + JSON.stringify(aMessage.data));
let msg = aMessage.data;
let mm = aMessage.target;
@ -832,7 +859,7 @@ let SettingsRequestManager = {
switch (aMessage.name) {
case "child-process-shutdown":
if (DEBUG) debug("Child process shutdown received.");
this.removeMessageManager(mm, aMessage.principal);
this.forceFinalizeChildLocksOOP(mm, aMessage.principal);
break;
case "Settings:RegisterForMessages":
if (!SettingsPermissions.hasSomeReadPermission(aMessage.principal)) {
@ -847,7 +874,7 @@ let SettingsRequestManager = {
this.removeObserver(mm);
break;
case "Settings:CreateLock":
if (DEBUG) debug("Received CreateLock for " + msg.lockID + " from " + aMessage.principal.origin);
if (DEBUG) debug("Received CreateLock for " + msg.lockID + " from " + aMessage.principal.origin + " window: " + msg.windowID);
// If we try to create a lock ID that collides with one
// already in the system, consider it a security violation and
// kill.
@ -857,7 +884,11 @@ let SettingsRequestManager = {
return;
}
this.settingsLockQueue.push(msg.lockID);
this.lockInfo[msg.lockID] = SettingsLockInfo(this.settingsDB, mm, msg.lockID, msg.isServiceLock);
this.lockInfo[msg.lockID] = SettingsLockInfo(this.settingsDB,
mm,
msg.lockID,
msg.isServiceLock,
msg.windowID);
break;
case "Settings:Get":
if (DEBUG) debug("Received getRequest from " + msg.lockID);