Closed Bug 1339951 Opened 7 years ago Closed 7 years ago

Improvements to mscom weak references

Categories

(Core :: IPC, defect)

Unspecified
Windows
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: bugzilla, Assigned: bugzilla)

References

Details

(Whiteboard: aes+)

Attachments

(1 file)

While testing the a11y smart proxy, I realized that the mscom weak reference stuff needs some improvements:

1) Deadlocks were possible due to lack of a lock hierarchy;
2) WeakReferenceSupport does not need an array of WeakRefs; since we are obtaining multiple weak references that always point to the same object, we only need one shared and thread-safe reference to the original object.

The following patch refactors the weakref stuff to get rid of that array and revise the locking scheme as follows:

1) There are two critical sections (using these due to reentrancy requirements): One for ThreadSafeQueryInterface (WeakReferenceSupport::mCSForQI) and one inside the shared reference (SharedRef::mCS).
2) If acquiring mCSForQI, it must always be taken before mCS.
3) SharedRef::mCS protects the raw, weak pointer, as well as the refcount in the WeakReferenceSupport class itself.
Comment on attachment 8837807 [details]
Bug 1339951: Refactor mscom weak reference support and establish lock hierarchy within;

https://reviewboard.mozilla.org/r/112806/#review115040
Attachment #8837807 - Flags: review?(jmathies) → review+
Pushed by aklotz@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/06cd86f16c70
Refactor mscom weak reference support and establish lock hierarchy within; r=jimm
Pushed by aklotz@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b68489018777
Refactor mscom weak reference support and establish lock hierarchy within; r=jimm
OK. Some minor changes that I made in a later patch should have been in this one. I fixed that and re-pushed.
Flags: needinfo?(aklotz)
No longer blocks: 1340733
https://hg.mozilla.org/mozilla-central/rev/b68489018777
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.