[PATCH] riched20: Release both the inner and outer IRichEditOle interface (Valgrind).
Signed-off-by: Sven Baars <sven.wine(a)gmail.com> --- dlls/riched20/richole.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/dlls/riched20/richole.c b/dlls/riched20/richole.c index dc1a3b8405..59204f54a9 100644 --- a/dlls/riched20/richole.c +++ b/dlls/riched20/richole.c @@ -1006,21 +1006,38 @@ static HRESULT WINAPI IRichEditOle_fnQueryInterface(IRichEditOle *me, REFIID riid, LPVOID *ppvObj) { IRichEditOleImpl *This = impl_from_IRichEditOle(me); - return IUnknown_QueryInterface(This->outer_unk, riid, ppvObj); + if (This->outer_unk) + return IUnknown_QueryInterface(This->outer_unk, riid, ppvObj); + else + return IUnknown_QueryInterface(&This->IUnknown_inner, riid, ppvObj); } static ULONG WINAPI IRichEditOle_fnAddRef(IRichEditOle *me) { IRichEditOleImpl *This = impl_from_IRichEditOle(me); - return IUnknown_AddRef(This->outer_unk); + + ULONG ref = IUnknown_AddRef(&This->IUnknown_inner); + if (This->outer_unk) + IUnknown_AddRef(This->outer_unk); + + TRACE ("%p ref=%u\n", This, ref); + + return ref; } static ULONG WINAPI IRichEditOle_fnRelease(IRichEditOle *me) { IRichEditOleImpl *This = impl_from_IRichEditOle(me); - return IUnknown_Release(This->outer_unk); + + if (This->outer_unk) + IUnknown_Release(This->outer_unk); + ULONG ref = IUnknown_Release(&This->IUnknown_inner); + + TRACE ("%p ref=%u\n", This, ref); + + return ref; } static HRESULT WINAPI @@ -5470,7 +5487,7 @@ LRESULT CreateIRichEditOle(IUnknown *outer_unk, ME_TextEditor *editor, LPVOID *p if (outer_unk) reo->outer_unk = outer_unk; else - reo->outer_unk = &reo->IUnknown_inner; + reo->outer_unk = NULL; *ppvObj = &reo->IRichEditOle_iface; return 1; -- 2.17.1
Signed-off-by: Sven Baars <sven.wine(a)gmail.com> --- dlls/ole32/compositemoniker.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/dlls/ole32/compositemoniker.c b/dlls/ole32/compositemoniker.c index 75d97fa7e3..94f4a723a8 100644 --- a/dlls/ole32/compositemoniker.c +++ b/dlls/ole32/compositemoniker.c @@ -433,6 +433,7 @@ static HRESULT WINAPI CompositeMonikerImpl_Reduce(IMoniker* iface, IBindCtx* pbc, DWORD dwReduceHowFar, IMoniker** ppmkToLeft, IMoniker** ppmkReduced) { + HRESULT res; IMoniker *tempMk,*antiMk,*rightMostMk,*leftReducedComposedMk,*rightMostReducedMk; IEnumMoniker *enumMoniker; @@ -453,7 +454,11 @@ CompositeMonikerImpl_Reduce(IMoniker* iface, IBindCtx* pbc, DWORD dwReduceHowFar IMoniker_ComposeWith(iface,antiMk,0,&tempMk); IMoniker_Release(antiMk); - return IMoniker_Reduce(rightMostMk,pbc,dwReduceHowFar,&tempMk, ppmkReduced); + res = IMoniker_Reduce(rightMostMk,pbc,dwReduceHowFar,&tempMk, ppmkReduced); + IMoniker_Release(tempMk); + IMoniker_Release(rightMostMk); + + return res; } else if (*ppmkToLeft==NULL) @@ -473,13 +478,16 @@ CompositeMonikerImpl_Reduce(IMoniker* iface, IBindCtx* pbc, DWORD dwReduceHowFar /* If any of the components reduces itself, the method returns S_OK and passes back a composite */ /* of the reduced components */ if (IMoniker_Reduce(rightMostMk,pbc,dwReduceHowFar,NULL,&rightMostReducedMk) && - IMoniker_Reduce(rightMostMk,pbc,dwReduceHowFar,&tempMk,&leftReducedComposedMk) - ) + IMoniker_Reduce(rightMostMk,pbc,dwReduceHowFar,&tempMk,&leftReducedComposedMk) ){ + IMoniker_Release(tempMk); + IMoniker_Release(rightMostMk); return CreateGenericComposite(leftReducedComposedMk,rightMostReducedMk,ppmkReduced); - + } else{ /* If no reduction occurred, the method passes back the same moniker and returns MK_S_REDUCED_TO_SELF.*/ + IMoniker_Release(tempMk); + IMoniker_Release(rightMostMk); IMoniker_AddRef(iface); -- 2.17.1
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=46956 Your paranoid android. === debian9 (32 bit Chinese:China report) === ole32: clipboard.c:1157: Test failed: got 800401d0 clipboard.c:1173: Test failed: got 800401d0 clipboard.c:1180: Test failed: data objects match clipboard.c:1187: Test failed: data objects match clipboard.c:1188: Test failed: data objects match clipboard.c:1198: Test failed: got 800401d0 clipboard.c:1210: Test failed: got 800401d0 clipboard.c:1211: Test failed: GetData not called clipboard.c:1214: Test failed: 1 1 clipboard.c:1220: Test failed: got 800401d0 clipboard.c:1221: Test failed: GetData not called clipboard.c:1225: Test failed: 1 clipboard.c:1236: Test failed: got 800401d0 clipboard.c:1247: Test failed: got 800401d0 clipboard.c:1253: Test failed: got 800401d0 clipboard.c:1254: Test failed: GetData not called clipboard.c:1260: Test failed: 1 1 clipboard.c:1265: Test failed: 1 clipboard.c:1278: Test failed: 1 1 clipboard.c:1282: Test failed: 1 === debian9 (64 bit WoW report) === ole32: clipboard.c:1049: Test failed: OleIsCurrentClipboard returned 0 clipboard.c:1117: Test failed: 1 WM_DRAWCLIPBOARD received
Summary from the discussions on irc: This just papers over the real bug. CreateIRichEditOle() is doing COM aggregation wrong. It needs to return the inner IUnknown when being aggregated. Of course the call site that does aggregation needs to be updated accordingly: dlls/riched20/txtsrv.c:84: if (!CreateIRichEditOle(This->outer_unk, This->editor, (void **)(&This->editor->reOle))) Plus other rippling effects as this is a nice COM cleanup after all ;) bye michael On 1/30/19 11:35 AM, Sven Baars wrote:
Signed-off-by: Sven Baars <sven.wine(a)gmail.com> --- dlls/riched20/richole.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/dlls/riched20/richole.c b/dlls/riched20/richole.c index dc1a3b8405..59204f54a9 100644 --- a/dlls/riched20/richole.c +++ b/dlls/riched20/richole.c @@ -1006,21 +1006,38 @@ static HRESULT WINAPI IRichEditOle_fnQueryInterface(IRichEditOle *me, REFIID riid, LPVOID *ppvObj) { IRichEditOleImpl *This = impl_from_IRichEditOle(me); - return IUnknown_QueryInterface(This->outer_unk, riid, ppvObj); + if (This->outer_unk) + return IUnknown_QueryInterface(This->outer_unk, riid, ppvObj); + else + return IUnknown_QueryInterface(&This->IUnknown_inner, riid, ppvObj); }
static ULONG WINAPI IRichEditOle_fnAddRef(IRichEditOle *me) { IRichEditOleImpl *This = impl_from_IRichEditOle(me); - return IUnknown_AddRef(This->outer_unk); + + ULONG ref = IUnknown_AddRef(&This->IUnknown_inner); + if (This->outer_unk) + IUnknown_AddRef(This->outer_unk); + + TRACE ("%p ref=%u\n", This, ref); + + return ref; }
static ULONG WINAPI IRichEditOle_fnRelease(IRichEditOle *me) { IRichEditOleImpl *This = impl_from_IRichEditOle(me); - return IUnknown_Release(This->outer_unk); + + if (This->outer_unk) + IUnknown_Release(This->outer_unk); + ULONG ref = IUnknown_Release(&This->IUnknown_inner); + + TRACE ("%p ref=%u\n", This, ref); + + return ref; }
static HRESULT WINAPI @@ -5470,7 +5487,7 @@ LRESULT CreateIRichEditOle(IUnknown *outer_unk, ME_TextEditor *editor, LPVOID *p if (outer_unk) reo->outer_unk = outer_unk; else - reo->outer_unk = &reo->IUnknown_inner; + reo->outer_unk = NULL; *ppvObj = &reo->IRichEditOle_iface;
return 1;
participants (3)
-
Marvin -
Michael Stefaniuc -
Sven Baars