mSingleton was a static member of the GetSingleton method, and was not getting cleared when the object was destroyed. Second attempts to create the object failed. Index: xpcprivate.h =================================================================== RCS file: /cvsroot/mozilla/js/src/xpconnect/src/xpcprivate.h,v retrieving revision 1.19 diff -u -r1.19 xpcprivate.h --- xpcprivate.h 1999/07/15 05:42:49 1.19 +++ xpcprivate.h 1999/07/31 08:17:48 @@ -860,6 +860,8 @@ // hide ctor and dtor nsXPCThreadJSContextStackImpl(); virtual ~nsXPCThreadJSContextStackImpl(); + static nsXPCThreadJSContextStackImpl* mSingleton; + }; Index: xpcthreadcontext.cpp =================================================================== RCS file: /cvsroot/mozilla/js/src/xpconnect/src/xpcthreadcontext.cpp,v retrieving revision 1.2 diff -u -r1.2 xpcthreadcontext.cpp --- xpcthreadcontext.cpp 1999/07/15 05:42:50 1.2 +++ xpcthreadcontext.cpp 1999/07/31 08:21:32 @@ -71,19 +71,24 @@ NS_ADDREF_THIS(); } -nsXPCThreadJSContextStackImpl::~nsXPCThreadJSContextStackImpl() {} +nsXPCThreadJSContextStackImpl::~nsXPCThreadJSContextStackImpl() +{ + mSingleton = nsnull; +} static NS_DEFINE_IID(knsXPCThreadJSContextStackImplIID, NS_IJSCONTEXTSTACK_IID); NS_IMPL_ISUPPORTS(nsXPCThreadJSContextStackImpl, knsXPCThreadJSContextStackImplIID); +nsXPCThreadJSContextStackImpl* nsXPCThreadJSContextStackImpl::mSingleton = nsnull; + //static nsXPCThreadJSContextStackImpl* nsXPCThreadJSContextStackImpl::GetSingleton() { - static nsXPCThreadJSContextStackImpl* singleton = NULL; - if(!singleton) - singleton = new nsXPCThreadJSContextStackImpl(); - return singleton; + if(!mSingleton) + mSingleton = + new nsXPCThreadJSContextStackImpl(); + return mSingleton; } /* readonly attribute PRInt32 Count; */

This patch doesn't cover it. I'm seeing failures while invoking QueryInterface on the 'second' instance. Program received signal SIGSEGV, Segmentation fault. 0x0 in ?? () Current language: auto; currently c (gdb) where #0 0x0 in ?? () #1 0x401088e8 in XPTC_InvokeByIndex (that=0x8074490, methodIndex=0, paramCount=2, params=0xbfffdd04) at xptcinvoke_unixish_x86.cpp:160 #2 0x402d1bef in nsXPCWrappedNativeClass::CallWrappedMethod (this=0x808a918, cx=0x8062fa8, wrapper=0x81369e8, desc=0x80929e0, callMode=CALL_METHOD, argc=1, argv=0x80814f8, vp=0xbfffdee8) at xpcwrappednativeclass.cpp:511 #3 0x402d34ef in WrappedNative_CallMethod (cx=0x8062fa8, obj=0x810b4d8, argc=1, argv=0x80814f8, vp=0xbfffdee8) at xpcwrappednativejsops.cpp:129 #4 0x40043f03 in js_Invoke (cx=0x8062fa8, argc=1, flags=0) at jsinterp.c:654 ... #12 0x804aa85 in main (argc=0, argv=0xbffff858) at xpcshell.cpp:568

reassigning to me. I'm a little confused, but interested... I'm not clear how or why you are accessing this particular class via JS. Its interface is (intentionally) not marked as scriptable. Are you confusing this with something else? What JS code is running when you hit the error you show? This service holds an important state even if no one is using it at a given point in time. I don't want to have the service manager release it on a whim. So, I will add an extra addref in xpconnect to keep it from being deleted unless someone convinces me otherwise.

I've got class and interface enumeration working in JavaScript on my local tree, and I've written a test script that creates an instance of every class, and querys it for every interface. After each successful QueryInterface, it enumerates all properties on the new object, and then releases the instance. This has flushed quite a few basic create/destroy without calling Init() and create/destroy/create bugs. So... I'm not really using nsXPCThreadJSContextStackImpl for any purpose other than smoke testing. Maybe the real bug is that this class is even showing up in JavaScript.

I should have figured that this was what you were up to. Very cool! When are you going to submit this stuff so that mccabe and I can get our grubby hands on it? Your patch in this instance would have been good except that, once created, I *really* don't want the object to ever be deleted in the lifetime of the process. The bug was that nsXPCThreadJSContextStackImpl::GetSingleton was not doing an addref. When accessed only as a service this was only getting called once and the addref from the ctor made it look ok. But, with multiple calls the reference counts were getting unbalanced. Even with your patch callers ended up calling Release on an object that has already deleted itself becasue its refcount had not previously been pushed high enough. Very much my fault. Thanks. I just checked in... Index: xpcthreadcontext.cpp =================================================================== RCS file: /cvsroot/mozilla/js/src/xpconnect/src/xpcthreadcontext.cpp,v retrieving revision 1.2 diff -r1.2 xpcthreadcontext.cpp 67a68,75 > /* > * This object holds state that we don't want to lose! > * > * The plan is that once created this object never goes away. We do an > * intentional extra addref at construction to keep it around even if no one > * is using it. > */ > 85a94,95 > if(singleton) > NS_ADDREF(singleton); ... you should abandon your suggested patch and update from the trunck. As to this *very* interesting issues of calling CreateInstance on every registered component and QIing each for all known interfaces... I suppose there is (now!) always the chance that someone might do this. First off, this is not really a huge security issue because we can restrict it using the nsIXPCSecurityManager interface in places (like content) where this isn't to be allowed. I had completely overlooked the fact that *all* xpcom objects are at least a little bit scriptable because they all expose a scriptable QueryInterface via inheritence. This is an important thing to remember. If we wanted to we could invent 'nsISupportsNoScript'. People could declare it as an interface's base in their idl files. The typelib system would show a non-scriptable QI and we could jigger the C++ headers with something like: #define nsISupportsNoScript nsISupports Or use a typedef. Other language mappings might not be so happy! This is probably moot. I think that the interesting 'damage' is already done in cases where components get created using CreateInstance where they expect to only be services, or vica versa. This remains a broken part of xpcom - objects can't enforce the 'mode' of their creation and use. If they are inherently singletons then their factories can always return the same instance, but this really breaks the semantics of CreateInstance. This comes up now and then and none of us has an answer that we are prepared to impose on the system. Or bad things can happen if the object is a 'private' (though registered) object and expects some special handshake that is never given after creation when created by 'outsiders'. I don't think that we can do anything about all this except tell people to not write bad components. If you register a factory for a thing with the component manager, then you are in the fish bowl (or fish bowel on a bad day:) The thing to do is exactly what rginda@ix.netcom.com is doing -- exercise the heck out of things and see what breaks then fix those things. Thanks! (now show us the code! :) John. P.S. rginda, I'm not marking this bug fixed. Please check that the crash is gone when you can and mark it 'fixed' if you think that appropriate. Thanks.

What if CreateInstance were to return NS_ERROR_NO_PUBLIC_CREATE (or some such error) for classes that are only meant to be created by 'friends.' Components 'in the know' would create the class using a static NS_NewSekretObject (nsISekret **retval) constructor. The nsISekret object could still be used as a retval from some other publicly creatable object, and would be able to enjoy all the benifits of being scriptable, without the creation issues. From my *gasp* VB experience, I recall setting a boolean property described as "Publicly Creatable" on classes. So I assume MS-COM already has some way of doing this. I will create a small VB test case using this property and see of I can't get some more details. BTW, I will retest this fix when I get my tree back in order :)

setting QA contact to Christine Begle -- here ya go