More folding, moving, and shaking.
authorBenjamin Smedberg <benjamin@smedbergs.us>
Sat, 26 Jul 2008 00:12:55 -0400
changeset 163 1124940b811904ccbf0aa91533a7184843788d7b
parent 162 3ae1ca70680d55840cbf5a60088ebaebfe2bddc5
child 164 fa6388d0a0da8b02a6de7c99bb09ca58d2dd2013
push id37
push userbsmedberg@mozilla.com
push dateSat, 26 Jul 2008 04:16:20 +0000
More folding, moving, and shaking.
NS_FORBIDDEN
NS_FORBIDDEN2
comptr-rewrite.patch
nscore-class-annotation-types
series
static-check-gc-attributes
xpconnect-finalizers2
xultemplates-gcobjects2
new file mode 100644
--- /dev/null
+++ b/NS_FORBIDDEN
@@ -0,0 +1,16 @@
+diff --git a/xpcom/base/nscore.h b/xpcom/base/nscore.h
+--- a/xpcom/base/nscore.h
++++ b/xpcom/base/nscore.h
+@@ -473,6 +473,12 @@ typedef PRUint32 nsrefcnt;
+ #define NS_UNLIKELY(x)  (!!(x))
+ #endif
+ 
++#if defined(__GNUC__) && __GNUC__ >= 4 && __GNUC_MINOR__ >= 3
++#define NS_FORBIDDEN(reason) __attribute__(error(reason))
++#else
++#define NS_FORBIDDEN(reason)
++#endif
++
+  /*
+   * If we're being linked as standalone glue, we don't want a dynamic dependency
+   * on NSPR libs, so we skip the debug thread-safety checks, and we cannot use
new file mode 100644
--- /dev/null
+++ b/NS_FORBIDDEN2
@@ -0,0 +1,12 @@
+diff --git a/xpcom/base/nscore.h b/xpcom/base/nscore.h
+--- a/xpcom/base/nscore.h
++++ b/xpcom/base/nscore.h
+@@ -474,7 +474,7 @@ typedef PRUint32 nsrefcnt;
+ #endif
+ 
+ #if defined(__GNUC__) && __GNUC__ >= 4 && __GNUC_MINOR__ >= 3
+-#define NS_FORBIDDEN(reason) __attribute__(error(reason))
++#define NS_FORBIDDEN(reason) __attribute__((error(reason)))
+ #else
+ #define NS_FORBIDDEN(reason)
+ #endif
--- a/comptr-rewrite.patch
+++ b/comptr-rewrite.patch
@@ -11461,17 +11461,17 @@ diff --git a/xpcom/glue/nsISupportsImpl.
 +#define NS_IMPL_THREADSAFE_ADDREF(_class)
 +#define NS_IMPL_THREADSAFE_RELEASE(_class)
  
  #define NS_IMPL_THREADSAFE_ISUPPORTS0(_class)                                 \
    NS_IMPL_THREADSAFE_ADDREF(_class)                                           \
 diff --git a/xpcom/glue/nsISupportsUtils.h b/xpcom/glue/nsISupportsUtils.h
 --- a/xpcom/glue/nsISupportsUtils.h
 +++ b/xpcom/glue/nsISupportsUtils.h
-@@ -62,6 +62,70 @@
+@@ -62,6 +62,72 @@
  #endif
  
  /**
 + * XPCOM classes must derive from GCObject or GCFinalizedObject. This
 + * class makes "new Class" work by allocating the class with the
 + * global XPCOM GC object.
 + */
 +class XPCOMGCObject
@@ -11524,25 +11524,27 @@ diff --git a/xpcom/glue/nsISupportsUtils
 +    static void operator delete(void *obj) NS_NO_FINALIZER
 +    {
 +        NS_GetGC()->Free(obj);
 +    }
 +
 +private:
 +    // creating arrays of finalized objects using new[] is not practical: the
 +    // outer allocation needs to finalize the inner object. Use nsTArray instead.
-+    static void* operator new[](size_t size) NS_FORBIDDEN;
-+    static void operator delete[](void *obj) NS_FORBIDDEN;
++    static void* operator new[](size_t size)
++        NS_FORBIDDEN("Can't create arrays of finalized objects.");
++    static void operator delete[](void *obj)
++        NS_FORBIDDEN("Can't create arrays of finalized objects.");
 +};
 +
 +/**
   * Macro for instantiating a new object that implements nsISupports.
   * Note that you can only use this if you adhere to the no arguments
   * constructor com policy (which you really should!).
-@@ -73,106 +137,24 @@
+@@ -73,106 +139,24 @@
      _result = new _type();                                                    \
    PR_END_MACRO
  
 -/**
 - * Macro for deleting an object that implements nsISupports.
 - * @param _ptr The object to delete.
 - */
 -#define NS_DELETEXPCOM(_ptr)                                                  \
deleted file mode 100644
--- a/nscore-class-annotation-types
+++ /dev/null
@@ -1,17 +0,0 @@
-diff --git a/xpcom/base/nscore.h b/xpcom/base/nscore.h
---- a/xpcom/base/nscore.h
-+++ b/xpcom/base/nscore.h
-@@ -491,9 +491,13 @@ typedef PRUint32 nsrefcnt;
- #ifdef NS_STATIC_CHECKING
- #define NS_STACK_CLASS __attribute__((user("NS_stack")))
- #define NS_FINAL_CLASS __attribute__((user("NS_final")))
-+#define NS_GC_TYPE     __attribute__((user("NS_GC")))
-+#define NS_NOGC_TYPE   __attribute__((user("NS_NoGC")))
- #else
- #define NS_STACK_CLASS
- #define NS_FINAL_CLASS
-+#define NS_GC_TYPE
-+#define NS_NOGC_TYPE
- #endif
- 
- /**
--- a/series
+++ b/series
@@ -12,18 +12,19 @@ tamarin-valgrind-annotations
 tamarin-double-mark-is-ok
 tamarin-uninline-fixedmalloc
 tamarin-gcc43-strict-warnings
 tamarin-bug-427030-v1.patch
 crashrepoter-notparallel
 core-refcounting
 template-hashtable-getters
 gfx-refcounting
+NS_FORBIDDEN
 64bit
-nscore-class-annotation-types
+static-check-gc-attributes
 psm-remove-init-isupports
 success-macros.patch
 no-standaloneglue.patch
 prerewrite_fixes.patch
 maybeweak-crap.patch
 remove-cycle
 comptr-rewrite.patch
 proxy-fixup.patch
@@ -123,17 +124,16 @@ supports-array-use-comarray
 zipreader-cache
 root-xptimanager
 threadmanager-shutdown-norecreate
 nsScanner-gc
 nsObjectMapEntry
 root-rdfstuff
 nsXHTMLParanoidFragmentSink
 multiple-comptrs
-static-check-gc-attributes
 nsRangeStore-allowonstack
 accessible-stack-classes
 widget-stack-classes
 psm-stack-classes
 spellcheck-stack-classes
 necko-stack-classes
 securebrowser-cast
 profilemigrator-immortal
@@ -199,8 +199,11 @@ views-unmanaged
 nsProtocolProxyService-FilterLink-gcobject
 TextRunExpiringCache-finalized
 nsFontCache-mContext-unmanaged
 nsJSONWriter-temp-finalizable
 NameSpaceDecl-mOwner-unmanaged
 xbl-gcobjects
 xul-gcobjects
 xultemplates-gcobjects
+NS_FORBIDDEN2
+xpconnect-finalizers2
+xultemplates-gcobjects2
--- a/static-check-gc-attributes
+++ b/static-check-gc-attributes
@@ -2,17 +2,17 @@ Add static checking of gc types and corr
 
 This doesn't yet re-implement finalizer correctness, thought it will now that we have a reasonable pretense of storage classes. Come to think of it, I should be more precise right now about non-managed classes with destructors... these need to be marked NS_GCOK
 * * *
 * * *
 
 diff --git a/config/config.mk b/config/config.mk
 --- a/config/config.mk
 +++ b/config/config.mk
-@@ -514,6 +514,7 @@ TREEHYDRA_MODULES = \
+@@ -512,6 +512,7 @@ TREEHYDRA_MODULES = \
  TREEHYDRA_MODULES = \
    $(topsrcdir)/xpcom/analysis/outparams.js \
    $(topsrcdir)/xpcom/analysis/stack.js \
 +  $(topsrcdir)/xpcom/analysis/gc.js \
    $(NULL)
  
  DEHYDRA_ARGS = \
 diff --git a/xpcom/analysis/gc.js b/xpcom/analysis/gc.js
@@ -859,52 +859,36 @@ diff --git a/xpcom/analysis/static-check
 +  if (type.type === undefined)
 +    throw Error("Analysis error: %s should be a decl but doesn't have a type".format(type));
 +
 +  return new DeclStorageClass(type);
 +}
 diff --git a/xpcom/base/nscore.h b/xpcom/base/nscore.h
 --- a/xpcom/base/nscore.h
 +++ b/xpcom/base/nscore.h
-@@ -487,17 +487,45 @@ typedef PRUint32 nsrefcnt;
-  *
-  * NS_STACK_CLASS: a class which must only be instantiated on the stack
-  * NS_FINAL_CLASS: a class which may not be subclassed
-+ * NS_GC_TYPE    : a class which must be GC-allocated
-+ * NS_NOGC_TYPE  : a class which must not be GC-allocated
-+ * NS_MAYBEGC    : A type which may be part of a GC object which is not
-+ *                 finalized. Classes/structs without explicit or implied
-+ *                 destructors (POD types) are automatically assumed to be
-+ *                 NS_MAYBEGC. The destructor of this type, if any, must be
-+ *                 finalizer safe.
-+ * NS_MAYBEGCFINALIZED: The destructor of such a type is GC-safe, and is not
-+ *                 optional: GC type which has a maybegcfinalized member must
-+ *                 be finalized.
-  */
+@@ -497,9 +497,29 @@ typedef PRUint32 nsrefcnt;
  #ifdef NS_STATIC_CHECKING
  #define NS_STACK_CLASS __attribute__((user("NS_stack")))
  #define NS_FINAL_CLASS __attribute__((user("NS_final")))
 +#define NS_MANAGED     __attribute__((user("NS_managed")))
- #define NS_GC_TYPE     __attribute__((user("NS_GC")))
- #define NS_NOGC_TYPE   __attribute__((user("NS_NoGC")))
-+#define NS_GCOK        __attribute__((user("NS_GCOK")))
++#define NS_GC_TYPE     __attribute__((user("NS_GC")))
++#define NS_NOGC_TYPE   __attribute__((user("NS_NoGC")))
 +#define NS_FINALIZER_NOT_REQUIRED __attribute__((user("NS_finalizer_not_required")))
 +#define NS_UNMANAGED   __attribute__((user("NS_unmanaged")))
-+#define NS_FINALIZER   __attribute__((user("NS_Finalizer")))
-+#define NS_NO_FINALIZER __attribute__((user("NS_No_Finalizer")))
++#define NS_FINALIZER   __attribute__((user("NS_finalizer")))
++#define NS_NO_FINALIZER __attribute__((user("NS_no_finalizer")))
 +#define NS_MALLOCATOR  __attribute__((user("NS_mallocator")))
 +#define NS_GCALLOCATOR __attribute__((user("NS_gcallocator")))
 +#define NS_GCFINALIZEDALLOCATOR __attribute__((user("NS_gcfinalizedallocator")))
  #else
  #define NS_STACK_CLASS
  #define NS_FINAL_CLASS
 +#define NS_MANAGED
- #define NS_GC_TYPE
- #define NS_NOGC_TYPE
-+#define NS_GCOK
++#define NS_GC_TYPE
++#define NS_NOGC_TYPE
 +#define NS_FINALIZER_NOT_REQUIRED
 +#define NS_UNMANAGED
 +#define NS_FINALIZER
 +#define NS_NO_FINALIZER
 +#define NS_MALLOCATOR
 +#define NS_GCALLOCATOR
 +#define NS_GCFINALIZEDALLOCATOR
  #endif
new file mode 100644
--- /dev/null
+++ b/xpconnect-finalizers2
@@ -0,0 +1,195 @@
+diff --git a/js/src/xpconnect/src/xpcwrappednative.cpp b/js/src/xpconnect/src/xpcwrappednative.cpp
+--- a/js/src/xpconnect/src/xpcwrappednative.cpp
++++ b/js/src/xpconnect/src/xpcwrappednative.cpp
+@@ -822,191 +822,6 @@ NS_INTERFACE_MAP_END_THREADSAFE
+ 
+ NS_IMPL_THREADSAFE_ADDREF(XPCWrappedNative)
+ NS_IMPL_THREADSAFE_RELEASE(XPCWrappedNative)
+-
+-/*
+- *  Wrapped Native lifetime management is messy!
+- *
+- *  - At creation we push the refcount to 2 (only one of which is owned by
+- *    the native caller that caused the wrapper creation).
+- *  - During the JS GC Mark phase we mark any wrapper with a refcount > 1.
+- *  - The *only* thing that can make the wrapper get destroyed is the
+- *    finalization of mFlatJSObject. And *that* should only happen if the only
+- *    reference is the single extra (internal) reference we hold.
+- *
+- *  - The wrapper has a pointer to the nsISupports 'view' of the wrapped native
+- *    object i.e... mIdentity. This is held until the wrapper's refcount goes
+- *    to zero and the wrapper is released.
+- *
+- *  - The wrapper also has 'tearoffs'. It has one tearoff for each interface
+- *    that is actually used on the native object. 'Used' means we have either
+- *    needed to QueryInterface to verify the availability of that interface
+- *    of that we've had to QueryInterface in order to actually make a call
+- *    into the wrapped object via the pointer for the given interface.
+- *
+- *  - Each tearoff's 'mNative' member (if non-null) indicates one reference
+- *    held by our wrapper on the wrapped native for the given interface
+- *    associated with the tearoff. If we release that reference then we set
+- *    the tearoff's 'mNative' to null.
+- *
+- *  - We use the occasion of the JavaScript GCCallback for the JSGC_MARK_END
+- *    event to scan the tearoffs of all wrappers for non-null mNative members
+- *    that represent unused references. We can tell that a given tearoff's
+- *    mNative is unused by noting that no live XPCCallContexts hold a pointer
+- *    to the tearoff.
+- *
+- *  - As a time/space tradeoff we may decide to not do this scanning on
+- *    *every* JavaScript GC. We *do* want to do this *sometimes* because
+- *    we want to allow for wrapped native's to do their own tearoff patterns.
+- *    So, we want to avoid holding references to interfaces that we don't need.
+- *    At the same time, we don't want to be bracketing every call into a
+- *    wrapped native object with a QueryInterface/Release pair. And we *never*
+- *    make a call into the object except via the correct interface for which
+- *    we've QI'd.
+- *
+- *  - Each tearoff *can* have a mJSObject whose lazily resolved properties
+- *    represent the methods/attributes/constants of that specific interface.
+- *    This is optionally reflected into JavaScript as "foo.nsIFoo" when "foo"
+- *    is the name of mFlatJSObject and "nsIFoo" is the name of the given
+- *    interface associated with the tearoff. When we create the tearoff's
+- *    mJSObject we set it's parent to be mFlatJSObject. This way we know that
+- *    when mFlatJSObject get's collected there are no outstanding reachable
+- *    tearoff mJSObjects. Note that we must clear the private of any lingering
+- *    mJSObjects at this point because we have no guarentee of the *order* of
+- *    finalization within a given gc cycle.
+- */
+-
+-void
+-XPCWrappedNative::FlatJSObjectFinalized(JSContext *cx)
+-{
+-    if(!IsValid())
+-        return;
+-
+-    // Iterate the tearoffs and null out each of their JSObject's privates.
+-    // This will keep them from trying to access their pointers to the
+-    // dying tearoff object. We can safely assume that those remaining
+-    // JSObjects are about to be finalized too.
+-
+-    XPCWrappedNativeTearOffChunk* chunk;
+-    for(chunk = &mFirstChunk; chunk; chunk = chunk->mNextChunk)
+-    {
+-        XPCWrappedNativeTearOff* to = chunk->mTearOffs;
+-        for(int i = XPC_WRAPPED_NATIVE_TEAROFFS_PER_CHUNK-1; i >= 0; i--, to++)
+-        {
+-            JSObject* jso = to->GetJSObject();
+-            if(jso)
+-            {
+-                NS_ASSERTION(JS_IsAboutToBeFinalized(cx, jso), "bad!");
+-                JS_SetPrivate(cx, jso, nsnull);
+-                to->JSObjectFinalized();
+-            }
+-
+-            // We also need to release any native pointers held...
+-            to->SetNative(nsnull);
+-            to->SetInterface(nsnull);
+-        }
+-    }
+-
+-    GetScope()->GetWrapperMap()->Remove(mFlatJSObject);
+-
+-    if(IsWrapperExpired())
+-    {
+-        GetScope()->GetWrappedNativeMap()->Remove(this);
+-
+-        XPCWrappedNativeProto* proto = GetProto();
+-
+-        if(mScriptableInfo &&
+-           (!HasProto() ||
+-            (proto && proto->GetScriptableInfo() != mScriptableInfo)))
+-        {
+-            delete mScriptableInfo;
+-            mScriptableInfo = nsnull;
+-        }
+-
+-        mMaybeScope = nsnull;
+-    }
+-
+-    // This makes IsValid return false from now on...
+-    mFlatJSObject = nsnull;
+-
+-    // Note that it's not safe to touch mNativeWrapper here since it's
+-    // likely that it has already been finalized.
+-}
+-
+-void
+-XPCWrappedNative::SystemIsBeingShutDown(JSContext* cx)
+-{
+-#ifdef DEBUG_xpc_hacker
+-    {
+-        printf("Removing root for still-live XPCWrappedNative %p wrapping:\n",
+-               static_cast<void*>(this));
+-        for(PRUint16 i = 0, i_end = mSet->GetInterfaceCount(); i < i_end; ++i)
+-        {
+-            nsXPIDLCString name;
+-            mSet->GetInterfaceAt(i)->GetInterfaceInfo()
+-                ->GetName(getter_Copies(name));
+-            printf("  %s\n", name.get());
+-        }
+-    }
+-#endif
+-    DEBUG_TrackShutdownWrapper(this);
+-
+-    if(!IsValid())
+-        return;
+-
+-    // The long standing strategy is to leak some objects still held at shutdown.
+-    // The general problem is that propagating release out of xpconnect at
+-    // shutdown time causes a world of problems.
+-
+-    // We leak mIdentity (see above).
+-
+-    // short circuit future finalization
+-    JS_SetPrivate(cx, mFlatJSObject, nsnull);
+-    mFlatJSObject = nsnull; // This makes 'IsValid()' return false.
+-
+-    XPCWrappedNativeProto* proto = GetProto();
+-
+-    if(HasProto())
+-        proto->SystemIsBeingShutDown(cx);
+-
+-    if(mScriptableInfo &&
+-       (!HasProto() ||
+-        (proto && proto->GetScriptableInfo() != mScriptableInfo)))
+-    {
+-        delete mScriptableInfo;
+-    }
+-
+-    // cleanup the tearoffs...
+-
+-    XPCWrappedNativeTearOffChunk* chunk;
+-    for(chunk = &mFirstChunk; chunk; chunk = chunk->mNextChunk)
+-    {
+-        XPCWrappedNativeTearOff* to = chunk->mTearOffs;
+-        for(int i = XPC_WRAPPED_NATIVE_TEAROFFS_PER_CHUNK-1; i >= 0; i--, to++)
+-        {
+-            if(to->GetJSObject())
+-            {
+-                JS_SetPrivate(cx, to->GetJSObject(), nsnull);
+-#ifdef XPC_IDISPATCH_SUPPORT
+-                if(to->IsIDispatch())
+-                    delete to->GetIDispatchInfo();
+-#endif
+-                to->SetJSObject(nsnull);
+-            }
+-            // We leak the tearoff mNative
+-            // (for the same reason we leak mIdentity - see above).
+-            to->SetNative(nsnull);
+-            to->SetInterface(nsnull);
+-        }
+-    }
+-
+-    if(mFirstChunk.mNextChunk)
+-    {
+-        delete mFirstChunk.mNextChunk;
+-        mFirstChunk.mNextChunk = nsnull;
+-    }
+-}
+-
+-/***************************************************************************/
+ 
+ // static
+ nsresult
new file mode 100644
--- /dev/null
+++ b/xultemplates-gcobjects2
@@ -0,0 +1,11 @@
+diff --git a/content/xul/templates/src/nsRuleNetwork.h b/content/xul/templates/src/nsRuleNetwork.h
+--- a/content/xul/templates/src/nsRuleNetwork.h
++++ b/content/xul/templates/src/nsRuleNetwork.h
+@@ -123,7 +123,6 @@ protected:
+ 
+         ~List() {
+             MOZ_COUNT_DTOR(MemoryElementSet::List);
+-            mElement->Destroy();
+             NS_IF_RELEASE(mNext); }
+ 
+         MemoryElement* mElement;