Last Comment Bug 34069 - js_DefineProperty stuck lock bug if JSPROP_[GS]ETTER and property exists in prototype object
: js_DefineProperty stuck lock bug if JSPROP_[GS]ETTER and property exists in p...
Status: RESOLVED FIXED
: js1.5
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
P3 normal (vote)
: M15
Assigned To: Brendan Eich [:brendan]
: Robert Ginda
: Steven DeTar [:sdetar]
Mentors:
: 34111 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2000-03-31 11:06 PST by Brendan Eich [:brendan]
Modified: 2002-01-18 00:39 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
proposed fix (581 bytes, patch)
2000-03-31 11:14 PST, Brendan Eich [:brendan]
no flags Details | Diff | Splinter Review

Description User image Brendan Eich [:brendan] 2000-03-31 11:06:37 PST
Summary says it all.  Patch coming up.

/be
Comment 1 User image Brendan Eich [:brendan] 2000-03-31 11:14:08 PST
Created attachment 7133 [details] [diff] [review]
proposed fix
Comment 2 User image Brendan Eich [:brendan] 2000-03-31 11:15:13 PST
Jband, feel free to check in with r=brendan@mozilla.org and close this.

/be
Comment 3 User image John Bandhauer 2000-03-31 11:44:41 PST
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 User image David Hyatt 2000-03-31 14:03:24 PST
*** Bug 34111 has been marked as a duplicate of this bug. ***
Comment 5 User image David Hyatt 2000-03-31 14:04:10 PST
Nominating for beta2.  This blocks the usage of the new XUL widgetry and 
therefore blocks skins.
Comment 6 User image Brendan Eich [:brendan] 2000-03-31 19:15:07 PST
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 User image Brendan Eich [:brendan] 2000-03-31 20:23:30 PST
Fix checked in.

/be

Note You need to log in before you can comment on or make changes to this bug.