Summary says it all. Patch coming up.
Created attachment 7133 [details] [diff] [review]
Jband, feel free to check in with firstname.lastname@example.org and close this.
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?
*** Bug 34111 has been marked as a duplicate of this bug. ***
Nominating for beta2. This blocks the usage of the new XUL widgetry and
therefore blocks skins.
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,
Fix checked in.