Closed
Bug 118194
Opened 23 years ago
Closed 23 years ago
Stale documentOwner pointers are left in the tree
Categories
(Core :: DOM: Core & HTML, defect, P2)
Core
DOM: Core & HTML
Tracking
()
VERIFIED
FIXED
mozilla0.9.9
People
(Reporter: rayw, Assigned: jst)
Details
(Whiteboard: [HAVE FIX])
Attachments
(1 file)
1.08 KB,
patch
|
bzbarsky
:
review+
vidur
:
superreview+
|
Details | Diff | Splinter Review |
I am taking Johnny's word on this one. Create an XML document (not the main document) and obtain a reference to a leaf node. Now, drop the reference to the document (ffter the garbage collector kicks in if you are in JS), and the references to the ownerDocument become stale because the document has been destroyed whereas individual nodes have not. Having Javascript have access to stale pointers makes it not safe or secure, because it is probably possible to write new contents into objects and even the vtable pointer since they are in the available memory pool and then use the object. Whenever a document is destroyed, it must explicitly zero the ownerDocument of all nodes it has ever created that are still being referenced (not just those currently in the hierarchy, because the script could be holding on to an node that was removed from the hierarchy). This affects SOAP because the user gets into these situations with documents that are not attached to a document window, such as those used by SOAP.
Assignee | ||
Comment 1•23 years ago
|
||
Assignee | ||
Comment 2•23 years ago
|
||
This is hard to trigger, and we don't have a testcase, but I'm sure it's a crash waiting to happen. The attached patch should make sure this crash can never happen.
Status: NEW → ASSIGNED
OS: Linux → All
Priority: -- → P2
Whiteboard: [HAVE FIX]
Target Milestone: --- → mozilla0.9.8
Comment 3•23 years ago
|
||
ray can you please provide the testcase.
Assignee | ||
Comment 4•23 years ago
|
||
I don't think there is a testcase for this, if we really need a testcase we could probably create one, but I don't think that's needed for this bug. We can verify this on the code level.
Reporter | ||
Comment 5•23 years ago
|
||
As I said before (unless I missed something) it is not enough to clear the documentOwner pointer on descendants, because the script may easily still have a pointer to an element that has been removed from underneath the hierarchy. All nodes which are still externally referenced must be cleared whether or not they are in the hierarchy.
Assignee | ||
Comment 6•23 years ago
|
||
Nodes that are not part of the document will not have a direct pointer back to the document. The document will be reachable from *elements* that are not part of the document as long as the document is alive, but that weak reference (through nsINodeInfo) is already properly cleared un document destruction, the last few lines in ~nsDocument() takes care of that: if (mNodeInfoManager) { mNodeInfoManager->DropDocumentReference(); } As for non-element nodes that are not part of a document, they don't have a reference back to the document any more. That is of course a conformance bug (which is filed already).
Assignee | ||
Updated•23 years ago
|
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Updated•23 years ago
|
Comment 7•23 years ago
|
||
Comment on attachment 63540 [details] [diff] [review] Clear the document pointer in all children on document destruction when needed. r=bzbarsky if you change the aCompileEventHandlers arg to PR_FALSE
Attachment #63540 -
Flags: review+
Comment 8•23 years ago
|
||
Comment on attachment 63540 [details] [diff] [review] Clear the document pointer in all children on document destruction when needed. sr=vidur with bzbarsky's required change.
Attachment #63540 -
Flags: superreview+
Assignee | ||
Comment 9•23 years ago
|
||
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Component: DOM: Core → DOM: Core & HTML
QA Contact: stummala → general
You need to log in
before you can comment on or make changes to this bug.
Description
•