Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-26 06:11:37 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1536154 - Eagerly run finalizer for any dead reflector JSObject when creating a new reflector for a DOM native r=bzbarsky

Browse Source 
Currently incremental GC can run the finalizer for a dead reflector for a native after a new reflector for that native has been created and attached. This leads to the confusing situation where there are two reflectors that contain pointers to the same native (which has a pointer to the new one).

This is a problem for memory accounting because the JS engine sees the size of the native at finalization time but does not see updates to this size after a new reflector is created. Thus the engine's idea of the size of a native can become incorrect and the memory accounting can become unbalanced.

Consider the following situation:

1. Native object created of size 20MB
2. Reflector 1 created
3. Reflector 1 becomes unreachable
4. Reflector 2 created
5. Native size changes to 40MB
6. Reflector 1 finalized

The memory associated with reflector 1 will be: 20MB (step 2), -20MB (step 6)

The memory associated with reflector 2 will be: 20MB (step 4), 40MB (step 5)

The memory associated with reflector 1 ends up negative (which should not be possible) and the total is also wrong.

The patch runs the finalizer for any dead reflector when creating a new one. This ensures that finalizer sees the correct state. The native object pointer is cleared when this happens so when the GC later runs the finalizer again it is a no-op. This situation occurs pretty rarely so I don't think there is much overhead to running the finalizer more than once. This also allows us to tighten up the assertions in the finalizer.

Differential Revision: https://phabricator.services.mozilla.com/D28690
This commit is contained in:
Jon Coppeard 2019-04-24 15:58:39 +01:00
parent bbbdd17fb2
commit b00f4011de
3 changed files with 27 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
dom/base/nsWrapperCacheInlines.h
View File

@ -14,9 +14,9 @@ inline JSObject* nsWrapperCache::GetWrapperPreserveColor() const {
JSObject* obj = GetWrapperMaybeDead();
if (obj && js::gc::EdgeNeedsSweepUnbarriered(&obj)) {
// The object has been found to be dead and is in the process of being
// finalized, so don't let the caller see it. As an optimisation, remove it
// from the cache so we don't have to do this check in future.
const_cast<nsWrapperCache*>(this)->ClearWrapper();
// finalized, so don't let the caller see it.
// Don't clear the cache though: this happens when a new wrapper is created
// for this native or when the wrapper is finalized.
return nullptr;
}
MOZ_ASSERT(obj == mWrapper);

6
dom/bindings/BindingUtils.h
View File

@ -1384,8 +1384,6 @@ inline mozilla::dom::ReflectionScope GetReflectionScope(
template <class T>
inline void ClearWrapper(T* p, nsWrapperCache* cache, JSObject* obj) {
JS::AutoAssertGCCallback inCallback;
// Skip clearing the wrapper when replaying. This method is called during
// finalization of |obj|, and when replaying a strong reference is kept on
// the contents of the cache: since |obj| is being finalized, the cache
@ -1394,13 +1392,15 @@ inline void ClearWrapper(T* p, nsWrapperCache* cache, JSObject* obj) {
// released, if we are finalizing later than we did while recording, and the
// cache may have already been deleted.
if (!recordreplay::IsReplaying()) {
MOZ_ASSERT(cache->GetWrapperMaybeDead() == obj);
cache->ClearWrapper(obj);
}
}
template <class T>
inline void ClearWrapper(T* p, void*, JSObject* obj) {
JS::AutoAssertGCCallback inCallback;
// QueryInterface to nsWrapperCache can't GC, we hope.
JS::AutoSuppressGCAnalysis nogc;
// Skip clearing the wrapper when replaying, for the same reason as in the
// overload above: |p| may have been deleted and we cannot QI it.

25
dom/bindings/Codegen.py
View File

@ -1730,7 +1730,7 @@ class CGAddPropertyHook(CGAbstractClassHook):
def finalizeHook(descriptor, hookName, freeOp, obj):
finalize = ""
finalize = "js::SetReservedSlot(%s, DOM_OBJECT_SLOT, JS::UndefinedValue());\n" % obj
if descriptor.interface.getExtendedAttribute('OverrideBuiltins'):
finalize += fill(
"""
@ -3847,6 +3847,11 @@ class CGWrapWithCacheMethod(CGAbstractMethod):
return false;
""")
if self.descriptor.proxy:
finalize = "DOMProxyHandler::getInstance()->finalize"
else:
finalize = FINALIZE_HOOK_NAME
return fill(
"""
static_assert(!IsBaseOf<NonRefcountedDOMObject, ${nativeType}>::value,
@ -3860,6 +3865,17 @@ class CGWrapWithCacheMethod(CGAbstractMethod):
MOZ_ASSERT(ToSupportsIsOnPrimaryInheritanceChain(aObject, aCache),
"nsISupports must be on our primary inheritance chain");
// If the wrapper cache contains a dead reflector then finalize that
// now, ensuring that the finalizer for the old reflector always
// runs before the new reflector is created and attached. This
// avoids the awkward situation where there are multiple reflector
// objects that contain pointers to the same native.
if (JSObject* oldReflector = aCache->GetWrapperMaybeDead()) {
${finalize}(nullptr /* unused */, oldReflector);
MOZ_ASSERT(!aCache->GetWrapperMaybeDead());
}
JS::Rooted<JSObject*> global(aCx, FindAssociatedGlobal(aCx, aObject->GetParentObject()));
if (!global) {
return false;
@ -3907,7 +3923,8 @@ class CGWrapWithCacheMethod(CGAbstractMethod):
createObject=CreateBindingJSObject(self.descriptor, self.properties),
unforgeable=CopyUnforgeablePropertiesToInstance(self.descriptor,
failureCode),
slots=InitMemberSlots(self.descriptor, failureCode))
slots=InitMemberSlots(self.descriptor, failureCode),
finalize=finalize)
class CGWrapMethod(CGAbstractMethod):
@ -12869,14 +12886,14 @@ class CGDOMJSProxyHandler_set(ClassMethod):
}
JS_MarkCrossZoneId(cx, id);
return dom::DOMProxyHandler::set(cx, proxy, id, wrappedValue, wrappedReceiver, result);
""")
class CGDOMJSProxyHandler_EnsureHolder(ClassMethod):
"""
Implementation of set(). We only use this for cross-origin objects.
Implementation of set(). We only use this for cross-origin objects.
"""
def __init__(self, descriptor):
args = [Argument('JSContext*', 'cx'),