Security Manager crashes when opened.

VERIFIED FIXED

Status

Core Graveyard
Security: UI
P3
blocker
VERIFIED FIXED
18 years ago
2 years ago

People

(Reporter: dougt, Assigned: David Hyatt)

Tracking

({smoketest})

1.0 Branch
smoketest

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Comment hidden (empty)
(Reporter)

Comment 1

18 years ago
Clicking on the lock icon, or selecting Tasks -> Personal Manager -> Security 
Manager will crash todays mozilla build. 

I get an assert here:

NTDLL! 77f7629c()
gc_root_marker(JSHashEntry * 0x035d1660, int 71, void * 0x00e81fe0) line 663 + 
35 bytes
JS_HashTableEnumerateEntries(JSHashTable * 0x010045f0, int (JSHashEntry *, int, 
void *)* 0x002cf746 gc_root_marker(JSHashEntry *, int, void *), void * 
0x00e81fe0) line 363 + 15 bytes
js_GC(JSContext * 0x033bc3e0) line 787 + 21 bytes
js_ForceGC(JSContext * 0x033bc3e0) line 678 + 9 bytes
JS_GC(JSContext * 0x033bc3e0) line 1161 + 9 bytes
nsJSContext::GC(nsJSContext * const 0x032daba0) line 913 + 13 bytes
GlobalWindowImpl::SetNewDocument(GlobalWindowImpl * const 0x032d9410, 
nsIDOMDocument * 0x00000000) line 258
DocumentViewerImpl::~DocumentViewerImpl() line 363
DocumentViewerImpl::`scalar deleting destructor'(unsigned int 1) + 15 bytes
DocumentViewerImpl::Release(DocumentViewerImpl * const 0x02981bf0) line 303 + 
154 bytes
nsCOMPtr<nsIContentViewer>::assign_assuming_AddRef(nsIContentViewer * 
0x00000000) line 447
nsCOMPtr<nsIContentViewer>::assign_with_AddRef(nsISupports * 0x00000000) line 
818
nsCOMPtr<nsIContentViewer>::operator=(nsIContentViewer * 0x00000000) line 557
nsDocShell::SetupNewViewer(nsDocShell * const 0x0337e2a0, nsIContentViewer * 
0x0382a070) line 2221
nsWebShell::SetupNewViewer(nsWebShell * const 0x0337e2a0, nsIContentViewer * 
0x0382a070) line 749 + 13 bytes
nsDocShell::CreateContentViewer(nsDocShell * const 0x0337e2a0, const char * 
0x0012fb48, int 0, nsIChannel * 0x03936370, nsIStreamListener * * 0x0012fb88) 
line 2109 + 24 bytes
nsWebShell::DoContent(nsWebShell * const 0x0337e3b0, const char * 0x0012fb48, 
int 0, const char * 0x10086dc8 gCommonEmptyBuffer, nsIChannel * 0x03936370, 
nsIStreamListener * * 0x0012fb88, int * 0x0012fb2c) line 1413 + 41 bytes
nsDocumentOpenInfo::DispatchContent(nsIChannel * 0x03936370, nsISupports * 
0x00000000) line 399 + 109 bytes
nsDocumentOpenInfo::OnStartRequest(nsDocumentOpenInfo * const 0x039379c0, 
nsIChannel * 0x03936370, nsISupports * 0x00000000) line 253 + 16 bytes
nsHTTPServerListener::FinishedResponseHeaders() line 801 + 48 bytes
nsHTTPServerListener::OnDataAvailable(nsHTTPServerListener * const 0x03986040, 
nsIChannel * 0x032afa64, nsISupports * 0x03936370, nsIInputStream * 0x03985f9c, 
unsigned int 0, unsigned int 862) line 309 + 8 bytes
nsOnDataAvailableEvent::HandleEvent(nsOnDataAvailableEvent * const 0x03987ee0) 
line 382 + 47 bytes
nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x03987e90) line 97 + 12 bytes
PL_HandleEvent(PLEvent * 0x03987e90) line 563 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x01000c90) line 508 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x95b608cc, unsigned int 49292, unsigned int 0, 
long 16780432) line 1018 + 9 bytes
USER32! 77e71820()

then it will abnormally terminate.
Keywords: smoketest

Comment 2

18 years ago
Created attachment 7147 [details] [diff] [review]
give this patch a try.
(Reporter)

Comment 3

18 years ago
Your fix works for the AlertCheck() dialogs, but this I still am crashing when I 
open up the Security Advisor.
(Reporter)

Comment 4

18 years ago
I am dying on:
JS_ASSERT(root_points_to_gcArenaPool);

david, chris, do you know what this could be?

Comment 5

18 years ago
How to I reproduce this? I installed psm from iPlanet using Nav 4.72 and it runs 
fine there. When I click on the lock icon in mozilla it just hangs in socket io.
(Reporter)

Comment 6

18 years ago
sure.

the easy way is to

1. with mozilla go here:
http://docs.iplanet.com/docs/manuals/psm/psm-mozilla/index.html

2. scroll to the bottom of the page, and click the 'Install' button for your 
platform.

Now, try it.

Comment 7

18 years ago
Oh, I see. The problem I was having seems to be that I can't run Nav 4.x and 
Mozilla at the same time while using psm.

So, I see that the pointer in questionhas a value of 0x00000020.

Either there is a root still with an address in an object that is now defunct or 
someone is putting a bogus pointer in a place where they have told Js there will 
be a gcthing.

I can dig some.

Comment 8

18 years ago
After about four bajillion assertions (XXX not threadsafe!) I was able to get to 
optionslink just fine.
(Reporter)

Comment 9

18 years ago
waterson, click on the lock icon.

Comment 10

18 years ago
hmm. i crash trying to leave a secure page. never mind.

Comment 11

18 years ago
The root's name is "nsXBLBinding::mScriptObject" so it comes from:
http://lxr.mozilla.org/seamonkey/source/layout/xbl/src/nsXBLBinding.cpp#1058
(Reporter)

Comment 12

18 years ago
Could the crash when leaving could be cause by the warning dialog that appeared 
and you dismissed??


Here is more information:

I am opening the window with these parameter:

menubar=no,height=%d,width=%d

note the menubar attribute.  I do not crash anymore when I replace it with 
something like:

chrome,dialog=no,all

I do crash, but mouse click do not respond correctly (ie,.they go to different 
windows.)
(Assignee)

Comment 13

18 years ago
If a XUL element weren't having its doc properly set to nsnull before it gets 
destroyed, then both its refs and the XBL binding refs would not be removed.  
Maybe I'm somehow not clearing out all binding refs, but the place to check in 
nsXBLBinding would be RemoveScriptReferences.

Comment 14

18 years ago
I can verify that this still happens with 1.259 of nsGlobalWindow.cpp from
jband, at least on linux.
(Reporter)

Comment 15

18 years ago
hyatt, RemoveScriptReferences is not even hit.  The exit happen while opening 
the new window.
(Reporter)

Comment 16

18 years ago
Hyatt, over to you.  

I think that you are using the mScriptObject multiple times (in different 
jscontexts?) without addreffing.  
Assignee: dougt → hyatt

Comment 17

18 years ago
I'm seeing this on the Mac, from a different code path. I'm going to look into it 
tonight.

Comment 18

18 years ago
nsXBLBinding objects are getting released without ever removing their roots. That 
is clear. I added the following code, to remove roots in the destructor if 
necessary:

Index: mozilla/layout/xbl/src/nsXBLBinding.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/xbl/src/nsXBLBinding.cpp,v
retrieving revision 1.27
diff -c -2 -r1.27 nsXBLBinding.cpp
*** nsXBLBinding.cpp	2000/03/31 10:27:12	1.27
--- nsXBLBinding.cpp	2000/04/01 06:38:00
***************
*** 48,51 ****
--- 48,52 ----
  #include "nsINameSpace.h"
  #include "nsJSUtils.h"
+ #include "nsIJSRuntimeService.h"
  
  // Event listeners
***************
*** 333,338 ****
--- 334,361 ----
  }
  
+ /**
+  * Removes a root inadvertently left over.
+  * This is really an error condition, but I've written
+  * this to prevent the JS GC from asserting.
+  */
+ static nsresult removeRoot(void* aSlot)
+ {
+ 	const char kJSRuntimeServiceProgID[] = "nsJSRuntimeService";
+ 	nsresult rv;
+ 	NS_WITH_SERVICE(nsIJSRuntimeService, runtimeService, 
kJSRuntimeServiceProgID, &rv);
+ 	if (NS_FAILED(rv)) return rv;
+ 	JSRuntime* rt;
+ 	rv = runtimeService->GetRuntime(&rt);
+ 	if (NS_FAILED(rv)) return rv;
+ 	return (JS_RemoveRootRT(rt, aSlot) ? NS_OK : NS_ERROR_FAILURE);
+ }
+ 
  nsXBLBinding::~nsXBLBinding(void)
  {
+   if (mScriptObject) {
+     removeRoot(&mScriptObject);
+     mScriptObject = nsnull;
+   }
+ 
    delete mAttributeTable;
  
***************
*** 958,961 ****
--- 981,985 ----
    if (mScriptObject) {
      aContext->RemoveReference((void*) &mScriptObject, mScriptObject);
+     mScriptObject = nsnull;
    }
  
(Reporter)

Comment 19

18 years ago
that is it: it fixes this bug.  can you check this in?  
(Assignee)

Comment 20

18 years ago
Hang on.  I think I know the real problem...
(Assignee)

Comment 21

18 years ago
beard, this is good error code to have.  You should add an assertion in the 
destructor that will assert if we get in there and mScriptObject is non-null.

I know why this is happening down one particular path (this path), but we should 
leave the assert in so that we'll be able to discover and patch the other 
bad paths that will no doubt crop up.

Comment 22

18 years ago
Created attachment 7156 [details]
A new assert before the gc root marker

Comment 23

18 years ago
To provoke this assert before the gc_root_marker. I opened mozilla on Win98 
build from  the tree (6:00 PST) and when the dialup dialog went up I closed the 
dialog without connecting to the net. Please note also how the webshells went 
up.
(Reporter)

Comment 24

18 years ago
david,  I do not want this broken for monday which we have a patch which seams 
to fix this problem.  What is your status.  What is wrong with the patch that 
beard attached?
(Assignee)

Comment 25

18 years ago
Nothing is wrong.  I said in the above comment "beard, this is good error code 
to have."  Check it in.  The only change I made was to add an assertion in the 
destructor so we can see a stack trace of the final release that frees the 
binding.

(Assignee)

Comment 26

18 years ago
Excellent.  Thanks for the trace, mielke.  It's asserting in the exact place I 
expected it to.  I have a fix that should obviate the need for beard's checkin, 
although I think beard's checkin should still go in to catch any other paths 
that cause the problem in the future (if there are any).

I'll just handle checking both beard's and my code in later today.  Will be 
fixed by Monday.
(Reporter)

Comment 27

18 years ago
I checked in beard's patch:

Checking in nsXBLBinding.cpp;
/cvsroot/mozilla/layout/xbl/src/nsXBLBinding.cpp,v  <--  nsXBLBinding.cpp
new revision: 1.29; previous revision: 1.28
done

Marking as fixed.  Hyatt, if there are other problems, please open up new bugs 
as now my security dialogs work.  

Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 28

18 years ago
Verified fixed.
Status: RESOLVED → VERIFIED

Comment 29

17 years ago
Mass changing Security:Crypto to PSM
Component: Security: Crypto → Client Library
Product: Browser → PSM
Version: other → 2.1

Comment 30

17 years ago
Mass changing Security:Crypto to PSM

Updated

13 years ago
Component: Security: UI → Security: UI
Product: PSM → Core

Updated

10 years ago
Version: psm2.1 → 1.0 Branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.