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

RESOLVED FIXED in M15

Status

()

P3
normal
RESOLVED FIXED
19 years ago
17 years ago

People

(Reporter: brendan, Assigned: brendan)

Tracking

({js1.5})

Trunk
js1.5
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

19 years ago
Summary says it all.  Patch coming up.

/be
(Assignee)

Updated

19 years ago
Status: NEW → ASSIGNED
Keywords: js1.5
Target Milestone: --- → M15
(Assignee)

Comment 1

19 years ago
Created attachment 7133 [details] [diff] [review]
proposed fix
(Assignee)

Comment 2

19 years ago
Jband, feel free to check in with r=brendan@mozilla.org and close this.

/be

Comment 3

19 years ago
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

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

Comment 5

19 years ago
Nominating for beta2.  This blocks the usage of the new XUL widgetry and 
therefore blocks skins.
Keywords: beta2
(Assignee)

Comment 6

19 years ago
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
(Assignee)

Comment 7

19 years ago
Fix checked in.

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED

Updated

19 years ago
Keywords: nsbeta2
You need to log in before you can comment on or make changes to this bug.