Closed Bug 600889 Opened 9 years ago Closed 9 years ago

TM: "Assertion failure: tree->ip != ip,"

Categories

(Core :: JavaScript Engine, defect, critical)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
Tracking Status
blocking2.0 --- betaN+
status1.9.2 --- unaffected
status1.9.1 --- unaffected

People

(Reporter: gkw, Assigned: igor)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [sg:critical] fixed-in-tracemonkey)

Attachments

(1 file)

uneval = function(){}
Function("\
  function zz(aa) {\
    if (aa) this.a = decodeURIComponent;\
    gc();\
    delete this.a\
  }\
  for each(c in [0, 0, 0, 0, 0, 0, 0, new Boolean(false), \
                  0, new Boolean(false), new Boolean(false), \"\"]) {\
    l=new zz(c)\
  }\
")()

asserts js debug shell on TM changeset 98c134cf59ef with -j at Assertion failure: tree->ip != ip,

Setting s-s because this involves gc.
blocking2.0: --- → ?
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   54494:b079aae53212
user:        Igor Bukanov
date:        Tue Sep 21 14:58:19 2010 +0200
summary:     bug 597736 - fixing TreeFragment leak. r=gal
Blocks: 597736
Assignee: general → igor
blocking2.0: ? → betaN+
Attached patch v1Splinter Review
In the bug 597736 I have missed that TraceMonitor::sweep() must abort the recording if it trashes any peer related to a fragment with dead gc things.
Attachment #480735 - Flags: review?(gal)
Comment on attachment 480735 [details] [diff] [review]
v1

I think we should factor out the purge-this-tree code and then call it for the recorder fragment instead of wedging these two things into each other.
(In reply to comment #3)
> I think we should factor out the purge-this-tree code and then call it for the
> recorder fragment instead of wedging these two things into each other.

I am not sure what do you mean here. The recorder contains just one of the peers, right? Yet any peer with dead fragment implies trashing all the peers. Or do you suggest to move the check for the recorder fragment into TrashTree and abort the recording there?
I mean that I don't want to this from inside the loop checking whether we have arrived at a specific fragment and instead just do it before the loop.
(In reply to comment #5)
> I mean that I don't want to this from inside the loop checking whether we have
> arrived at a specific fragment and instead just do it before the loop.

AFAICS recorder's fragment with dead GC things is special since trashing it does not imply trashing other peers (recording can just be aborted) while recorded peers with dead things mean trashing all the peers. I do not see how to factor that out.
Whiteboard: [sg:critical]
gal: could you comment on the above
gal?
gal, double ping?
(In reply to comment #5)
> I mean that I don't want to this from inside the loop checking whether we have
> arrived at a specific fragment and instead just do it before the loop.

So is that an "r-" then? Trying to figure out whose court the ball is in here: waiting on review, or waiting on a new patch?
gal, triple ping. :-) I think this bug is stalled until you + or - the patch.
Attachment #480735 - Flags: review?(gal) → review+
Whiteboard: [sg:critical] → [sg:critical] fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/5177ee4c10d6
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Duplicate of this bug: 600884
Group: core-security
JSBugMon: This bug has been automatically verified fixed.
Status: RESOLVED → VERIFIED
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.