<html>
<head>
<base href="https://bugs.documentfoundation.org/">
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Crash using "Close" button for Extension Manager -> Check for Updates dialog"
href="https://bugs.documentfoundation.org/show_bug.cgi?id=115420#c42">Comment # 42</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Crash using "Close" button for Extension Manager -> Check for Updates dialog"
href="https://bugs.documentfoundation.org/show_bug.cgi?id=115420">bug 115420</a>
from <span class="vcard"><a class="email" href="mailto:glogow@fbihome.de" title="Jan-Marek Glogowski <glogow@fbihome.de>"> <span class="fn">Jan-Marek Glogowski</span></a>
</span></b>
<pre>(In reply to Michael Meeks from <a href="show_bug.cgi?id=115420#c39">comment #39</a>)
<span class="quote">> Hmm - then again ... looking at the code:
>
> vcl/win/window/salframe.cxx
>
> It puzzles me:
>
> WinSalFrame::~WinSalFrame()
> {
>
> // Release Cache DC
> if ( mpGraphics2 &&
> mpGraphics2->getHDC() )
> ReleaseGraphics( mpGraphics2 );</span >
That's fine - should be sufficient to just check for mpGraphics2, as
ReleaseGraphics does the rest.
<span class="quote">> Doesn't seem to do anything of the sort; unless mpGraphics2 == mpGraphics.
>
> Indeed the condition:
>
> void WinSalFrame::ReleaseGraphics( SalGraphics* pGraphics )
> {
> if ( mpGraphics2 == pGraphics )
> {
>
> Looks deeply counter-intuitive; surely that should be mpGraphics2 !=
> pGraphics - ie. only free it if it is different ?</span >
No - we just free the cached / shared / threaded DC (not the object), if we
want to release it.
<span class="quote">> I'm left puzzled by who frees mpGraphics2 - which (after all) has this timer
> associated with it. Of course, that is not related to GL, and perhaps is a
> leak but only when used threaded:
>
> WinSalGraphics* mpGraphics; // current frame graphics
> WinSalGraphics* mpGraphics2; // current frame
> graphics for other threads
>
> The rather grim extensions code being high on the awful threaded, graphical
> code list of horror.
>
> Thoughts much appreciated; I append a blind fix in case someone can
> reproduce this and can compile that.
>
> diff --git a/vcl/win/window/salframe.cxx b/vcl/win/window/salframe.cxx
> index 64b073f99139..fee8440cc700 100644
> --- a/vcl/win/window/salframe.cxx
> +++ b/vcl/win/window/salframe.cxx
> @@ -926,6 +926,17 @@ WinSalFrame::~WinSalFrame()
> mpGraphics2->getHDC() )
> ReleaseGraphics( mpGraphics2 );
>
> + // Why did we never do this ?
> + if (mpGraphics2 != mpGraphics)</span >
This is always true. Don't confuse the mpGraphics with pGraphics. I did it
quite some times when reading the code.
But we seem to leak the mpGraphics2 object, which is assigned once, but never
freed.
I'm now definitely preferring an *m_* prefix for member variables.
(In reply to Aron Budea from <a href="show_bug.cgi?id=115420#c40">comment #40</a>)
<span class="quote">> (In reply to Michael Meeks from <a href="show_bug.cgi?id=115420#c39">comment #39</a>)
> After applying the patch, I'm getting assertion failed at startup:
> "vcl/win/app/salint.cxx Line: 611
>
> Expression: !pInst->mbNoYieldLock"</span >
Probably we need something like the fix in
4baec725e0dc0713f0d47003e9b10bc3b62f56ff here.
Seems we switch to the main thread and then to an other thread again here.
But that is just a guess.
commit 4baec725e0dc0713f0d47003e9b10bc3b62f56ff
Author: Jan-Marek Glogowski <<a href="mailto:glogow@fbihome.de">glogow@fbihome.de</a>>
Date: Mon Aug 28 19:58:32 2017 +0200
WIN run main thread redirects ignoring SolarMutex</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>