34069 - js_DefineProperty stuck lock bug if JSPROP_[GS]ETTER and property exists in prototype object

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Brendan Eich [:brendan]]

Summary says it all.  Patch coming up.

/be

Comment 1

[Brendan Eich [:brendan]]

Attachment: proposed fix

Comment 2

[Brendan Eich [:brendan]]

Jband, feel free to check in with r=[e-mail] and close this.

/be

Comment 3

[John Bandhauer]

Brendan, looking at the loop in js_LookupProperty I still have not convinced 
myself that some object with a long proto chain might not leave the function 
with extraneous locks on the non-terminal nodes in the chain. But there is lots 
going on there that I don't quite get. Is there no way that this can happen?

Comment 4

[David Hyatt]

*** Bug 34111 has been marked as a duplicate of this bug. ***

Comment 5

[David Hyatt]

Nominating for beta2.  This blocks the usage of the new XUL widgetry and 
therefore blocks skins.

Comment 6

[Brendan Eich [:brendan]]

Jband: rest assured we'd see more stuck locks.  The bug in js_DefineProperty was 
added when I added getters and setters late last year, and it bit hyatt only the 
other night, when he started defining getters and setters for properties that 
shadow prototype properties (hyatt, what are some example id's of such props?).

The loop in js_LookupProperty has these exits:

1.  The false return after newResolve failure, but obj has been unlocked before 
the call to newResolve (a good idea in general, to avoid deadlocks).

2.  Likewise for "old-style" resolve's call in the else clause.

3.  The true return when a property has been found, in which case *by 
definition* the object owning that property should be locked, and it does in 
fact remain locked; and the property's refcount is incremented.  The caller is 
responsible for calling OBJ_DROP_PROPERTY later, to release the prop and unlock 
the object in whose scope it lives.

4.  The break after finding a null proto link, which terminates the prototype 
chain -- note again that the current object (whose proto was null) is unlocked 
just before the if(!proto)break.

5.  The immediately following call out to another JSObjectOps 
(OBJ_LOOKUP_PROPERTY) if the proto-object is not native.  Tail recursion after 
obj has been unlocked just above exit-point 4 (above).

The loop then sets obj = proto and iterates.  If this isn't a convincing proof 
I'll burn the weekend constructing a denotational semantic proof!

Anyway, to repeat: the bug was in the JS_HAS_GETTER_SETTER-#ifdef'd code in 
js_DefineProperty, and only there.

Is this fix in yet?  If so, close the bug or tell me to close it.  Thanks,

/be

Comment 7

[Brendan Eich [:brendan]]

Fix checked in.

/be