213482 - setting a watched property within the function called by a watch results in crash

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Brant Gurganus]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5a) Gecko/20030714 Mozilla Firebird/0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5a) Gecko/20030714 Mozilla Firebird/0.6

I had an input element on a page whose value property I was watching.  However,
I was new to using the watch method and was setting the value property from
within the function called by the watch method.  This (now) obviously leads to
an infinite loop which caused Firebird to crash.

Reproducible: Always

Steps to Reproduce:
1. Set the value property of an input element from the function called by the
watch method that is watching that property.

Actual Results:  
Firebird crashed.

Expected Results:  
Firebird should not crash.

Here is a minimal case:
<input id="test">

<script type="text/html">
test = document.getElementById('test');

function watched (a, b, c) {
    test.value = 'a';
}

test.watch("value", watched);
</script>

Comment 1

[Brant Gurganus]

The only other bug I found relating to the watch method is Bug 127243.

Comment 2

[Brant Gurganus]

Attachment: testcase

I e-mailed this file to myself in Thunderbird with JS on and it also crashes
there so this definitely is not Firebird-specific.

I'd give a Talkback ID, but it hasn't gone through yet.

Comment 3

[Phil Schwartau]

Attachment: Talkack stack trace

Comment 4

[Phil Schwartau]

cc'ing Brendan, Mike -

Comment 5

[Brendan Eich [:brendan]]

.

Comment 6

[Brant Gurganus]

Here are three Talkback IDs if they are useful, although we already have a stack
trace attached.

TB22108259X
TB22108009W
TB22107993H

Additionally, Windows did not appear to catch this crash, only Talkback caught
it if that narrows things down.

Comment 7

[Phil Schwartau]

Brant: thank you for these extra incident IDs. I looked them up,
and they confirm your original stack trace attached in Comment #3.
They all show the same call patterns leading to stack overflow -

Comment 8

[Brendan Eich [:brendan]]

Oops, just need some recursion dampening.  Path soon.

/be

Comment 9

[Brendan Eich [:brendan]]

Attachment: proposed fix

Also fixes a bug calling GC from a JSClass.resolve hook that scole has worked
around for a while now.

/be

Comment 10

[Steven Cole]

Comment on attachment 129039 [details] [diff] [review]
proposed fix

You're stretching the limits of my understanding of how the engine works... but
ok.  The scheme is understandable, and the code looks clean.  r=scole

Comment 11

[Brendan Eich [:brendan]]

Fixed, thanks.

/be

Comment 12

[Bob Clary [:bc] (inactive)]

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

Comment 13

[Bob Clary [:bc] (inactive)]

verified fixed