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

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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

Comment 2

[Igor Bukanov]

Attachment: v1

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.

Comment 3

[Andreas Gal :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.

Comment 4

[Igor Bukanov]

(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?

Comment 5

[Andreas Gal :gal]

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.

Comment 6

[Igor Bukanov]

(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.

Comment 7

[Igor Bukanov]

gal: could you comment on the above

Comment 8

[Johnny Stenback (:jst)]

gal?

Comment 9

[Johnny Stenback (:jst)]

gal, double ping?

Comment 10

[Daniel Veditz [:dveditz]]

(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?

Comment 11

[David Mandelin [:dmandelin]]

gal, triple ping. :-) I think this bug is stalled until you + or - the patch.

Comment 12

[Igor Bukanov]

http://hg.mozilla.org/tracemonkey/rev/5177ee4c10d6

Comment 13

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/5177ee4c10d6

Comment 15

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 16

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929