355339 - "Assertion failure: sprop->setter != js_watch_set" setting watch after unwatch

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Jesse Ruderman]

js> o = {}
[object Object]
js> o.watch("j", function(a,b,c) { print("*",a,b,c) })
js> o.unwatch("j")
js> o.watch("j", function(a,b,c) { print("*",a,b,c) })
Assertion failure: sprop->setter != js_watch_set, at jsdbgapi.c:541

Seems harmless in an opt jsshell.

Comment 1

[Jesse Ruderman]

I can still reproduce this bug.

Comment 2

[Jesse Ruderman]

I think I hit something similar to this with the fuzzer in bug 349611 today.

Comment 3

[Brendan Eich [:brendan]]

Attachment: fix

Comment 4

[Brendan Eich [:brendan]]

This isn't just a bogus assertion.  In an unpatched optimized shell, this modified version of the testcase:

o = {}
o.watch("j", function(a,b,c) { print("*",a,b,c) })
o.j = 1
o.unwatch("j")
o.j = 2
o.watch("j", function(a,b,c) { print("*",a,b,c) })
o.j = 3

produces this output (abridged):

* j undefined 1
* j 2 3
* j 2 undefined
. . .
* j 2 undefined
watch.js:7: InternalError: too much recursion

The fix is safe and pretty clearly correct.  The saved wp->sprop was wrong before, it pointed to the sprop with the original getter and setter, not the one cloned from sprop but with js_watch_set as the setter.  The mutated clone is the one to keep in wp->sprop, or else the DropWatchPoint code won't change the mapping in wp->object's scope back to the original sprop.

/be

Comment 5

[Brendan Eich [:brendan]]

Fixed on the trunk:

Checking in jsdbgapi.c;
/cvsroot/mozilla/js/src/jsdbgapi.c,v  <--  jsdbgapi.c
new revision: 3.73; previous revision: 3.72
done

/be

Comment 6

[Bob Clary [:bc] (inactive)]

RCS file: /cvsroot/mozilla/js/tests/js1_5/Regress/regress-355339.js,v
done
Checking in regress-355339.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-355339.js,v  <--  regress-355339.js
initial revision: 1.1
done

Comment 7

[Jay Patel [:jay]]

Comment on attachment 246120 [details] [diff] [review]
fix

Approved for both branches.  a=jay for drivers, please land on the 1.8.0 and 1.8.1 branches asap.  Thanks!

Comment 8

[Brendan Eich [:brendan]]

Fixed on the 1.8 branch:

Checking in jsdbgapi.c;
/cvsroot/mozilla/js/src/jsdbgapi.c,v  <--  jsdbgapi.c
new revision: 3.56.2.5; previous revision: 3.56.2.4
done

and the 1.8.0 branch:

Checking in jsdbgapi.c;
/cvsroot/mozilla/js/src/jsdbgapi.c,v  <--  jsdbgapi.c
new revision: 3.56.2.1.4.2; previous revision: 3.56.2.1.4.1
done

/be

Comment 9

[Bob Clary [:bc] (inactive)]

verified fixed 20061122 1.8.0.9 windows/linux/mac*, 1.8.1.1 windows/linux/mac*, 1.9 windows/linux