Closed Bug 1395329 Opened 5 years ago Closed 5 years ago

mscom::Interceptor::Create needs to handle IUnknown properly

Categories

(Core :: Disability Access APIs, enhancement)

Unspecified
Windows
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: bugzilla, Assigned: bugzilla)

References

Details

Attachments

(1 file, 1 obsolete file)

We're hitting an assertion on debug builds because of this. It's a correctness issue.
We're basically having problems any time an IUnknown is supplied as an outparam. This is likely to explain some of our crashiness as well.
Blocks: 1395059
Attached patch Fix (obsolete) — Splinter Review
IUnknown is a special case when creating interceptors, because the "intercepted" IUnknown is actually just the IUnknown of the interceptor itself.

The main part of this patch adds a special code path for IUnknown that still performs the essential duties of publishing the target IUnknown to the interceptor live set, and saving a strong reference to the target in mInterceptorMap.

But then we just QI ourselves for the output interface.


I also added some additional locking around this code, as it is possible for other threads to attempt to access an Interceptor once it has been published to the live set.
Attachment #8902931 - Flags: review?(jmathies)
Attached patch Fix (r2)Splinter Review
Cleaned this up a bit more
Attachment #8902931 - Attachment is obsolete: true
Attachment #8902931 - Flags: review?(jmathies)
Attachment #8902965 - Flags: review?(jmathies)
Attachment #8902965 - Flags: review?(jmathies) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/0eb505a8ac7ced3a94604d7581eb24f4a5b9a7c8
Bug 1395329: Modify mscom::Interceptor::Create to properly initialize IUnknown interfaces; r=jimm
Backed out for Windows static bustage in Interceptor.obj:

https://hg.mozilla.org/integration/mozilla-inbound/rev/075a61e7c9eba9f24dd0b11d0fc07cdb1a5f1b75

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=0eb505a8ac7ced3a94604d7581eb24f4a5b9a7c8&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=127800791&repo=mozilla-inbound

17:34:43     INFO -  clang.exe: warning: unknown argument ignored in clang-cl: '-deps.deps/Interceptor.obj.pp' [-Wunknown-argument]
17:34:43     INFO -  In file included from z:/build/build/src/ipc/mscom/Interceptor.cpp:11:
17:34:43     INFO -  In file included from z:/build/build/src/obj-firefox/dist/include\mozilla/mscom/DispatchForwarder.h:12:
17:34:43     INFO -  In file included from z:/build/build/src/obj-firefox/dist/include\mozilla/mscom/Interceptor.h:15:
17:34:43     INFO -  z:/build/build/src/obj-firefox/dist/include\mozilla/mscom/WeakRef.h(109,16):  error: typedef 'AutoLock' cannot be referenced with a class specifier
17:34:43     INFO -    friend class AutoLock;
17:34:43     INFO -                 ^
17:34:43     INFO -  z:/build/build/src/obj-firefox/dist/include\mozilla/mscom/WeakRef.h(108,46):  note: declared here
17:34:43     INFO -    typedef BaseAutoLock<WeakReferenceSupport> AutoLock;
17:34:43     INFO -                                               ^
17:34:43     INFO -  In file included from z:/build/build/src/ipc/mscom/Interceptor.cpp:9:
17:34:43     INFO -  In file included from z:/build/build/src/obj-firefox/dist/include\mozilla/dom/ContentChild.h:12:
17:34:43     INFO -  In file included from z:/build/build/src/obj-firefox/dist/include\mozilla/dom/ContentBridgeParent.h:10:
17:34:43     INFO -  In file included from z:/build/build/src/obj-firefox/ipc/ipdl/_ipdlheaders\mozilla/dom/PContentBridgeParent.h:9:
17:34:43     INFO -  In file included from z:/build/build/src/obj-firefox/ipc/ipdl/_ipdlheaders\mozilla/dom/PContentBridge.h:15:
17:34:43     INFO -  In file included from z:/build/build/src/obj-firefox/dist/include\mozilla/ipc/ProtocolUtils.h:27:
17:34:43     INFO -  In file included from z:/build/build/src/obj-firefox/dist/include\mozilla/MozPromise.h:13:
17:34:43     INFO -  z:/build/build/src/obj-firefox/dist/include/mozilla/Mutex.h(166,12):  error: 'Lock' is a protected member of 'mozilla::mscom::WeakReferenceSupport'
17:34:43     INFO -      mLock->Lock();
17:34:43     INFO -             ^
17:34:43     INFO -  z:/build/build/src/ipc/mscom/Interceptor.cpp(412,14):  note: in instantiation of member function 'mozilla::BaseAutoLock<mozilla::mscom::WeakReferenceSupport>::BaseAutoLock' requested here
17:34:43     INFO -      AutoLock lock(*this);
17:34:43     INFO -               ^
17:34:43     INFO -  z:/build/build/src/obj-firefox/dist/include\mozilla/mscom/WeakRef.h(105,8):  note: declared protected here
17:34:43     INFO -    void Lock();
17:34:43     INFO -         ^
17:34:43     INFO -  In file included from z:/build/build/src/ipc/mscom/Interceptor.cpp:9:
17:34:43     INFO -  In file included from z:/build/build/src/obj-firefox/dist/include\mozilla/dom/ContentChild.h:12:
17:34:43     INFO -  In file included from z:/build/build/src/obj-firefox/dist/include\mozilla/dom/ContentBridgeParent.h:10:
17:34:43     INFO -  In file included from z:/build/build/src/obj-firefox/ipc/ipdl/_ipdlheaders\mozilla/dom/PContentBridgeParent.h:9:
17:34:43     INFO -  In file included from z:/build/build/src/obj-firefox/ipc/ipdl/_ipdlheaders\mozilla/dom/PContentBridge.h:15:
17:34:43     INFO -  In file included from z:/build/build/src/obj-firefox/dist/include\mozilla/ipc/ProtocolUtils.h:27:
17:34:43     INFO -  In file included from z:/build/build/src/obj-firefox/dist/include\mozilla/MozPromise.h:13:
17:34:43     INFO -  z:/build/build/src/obj-firefox/dist/include/mozilla/Mutex.h(171,12):  error: 'Unlock' is a protected member of 'mozilla::mscom::WeakReferenceSupport'
17:34:43     INFO -      mLock->Unlock();
17:34:43     INFO -             ^
17:34:43     INFO -  z:/build/build/src/ipc/mscom/Interceptor.cpp(412,14):  note: in instantiation of member function 'mozilla::BaseAutoLock<mozilla::mscom::WeakReferenceSupport>::~BaseAutoLock' requested here
17:34:43     INFO -      AutoLock lock(*this);
17:34:43     INFO -               ^
17:34:43     INFO -  z:/build/build/src/obj-firefox/dist/include\mozilla/mscom/WeakRef.h(106,8):  note: declared protected here
17:34:43     INFO -    void Unlock();
17:34:43     INFO -         ^
17:34:43     INFO -  3 errors generated.
17:34:43     INFO -  z:/build/build/src/config/rules.mk:1064: recipe for target 'Interceptor.obj' failed
17:34:43     INFO -  mozmake.EXE[5]: *** [Interceptor.obj] Error 1
Flags: needinfo?(aklotz)
https://hg.mozilla.org/integration/mozilla-inbound/rev/be6e65eab263f4aba756a7f09bb8fb75fb818e21
Bug 1395329: Modify mscom::Interceptor::Create to properly initialize IUnknown interfaces; r=jimm
Yet another MSVC <--> clang annoyance.
Flags: needinfo?(aklotz)
https://hg.mozilla.org/mozilla-central/rev/be6e65eab263
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Duplicate of this bug: 1386486
Duplicate of this bug: 1381943
Duplicate of this bug: 1382087
You need to log in before you can comment on or make changes to this bug.