locking foo ...

[Michael Stahl]

On 03/12/13 22:33, Michael Meeks wrote:
> Hi Michael,
> 
> On Tue, 2013-12-03 at 17:03 +0100, Michael Stahl wrote:
>> also: the hypothesis that duplicating the state in CMAccessible prevents
>> locking SolarMutex in COM calls is not entirely true: for example
>> CMAccessible::get_accChildCount retrieves it from the XAccessibleContext
>> (... which it retrieves from m_xAccessible, ignoring m_xContext?).
>> similar code handles CHILDID_SELF cases throughout.
> 
> 	Good to know =)
> 
>> [ by the way i haven't done anything about the state duplication across
>> classes yet - just cleaned up cases where class had 2 members pointing
>> to the same thing. ]
> 
> 	Ok; fair enough.
> 
>> there are basically 2 different interfaces in CMAccessible: the
>> IAccessible2 ones will only be called from outside, while the internal
>> ones will only be called from UNO event listeners.  the internal ones
>> don't need any locking since the UNO event listeners have to do that
>> already.
> 
> 	Right - so I was overlooking the fact that native C++ calls know
> nothing about apartments and thus we can circumvent all that fluff and
> not care anything about it =) Indeed - I think we should be able to
> avoid CoCreateInstance completely:
> 
> http://msdn.microsoft.com/en-us/library/windows/desktop/ff485840%28v=vs.85%29.aspx
> 
> 	Suggests that just deriving from and working with IUnknown should be
> acceptable rather than going through the CoCreateInstance
> over-complexity...
> 
>> i think probably it's better in the current situation not to have the
>> components live in the main thread STA in the first place, and hope that
>> just creating them with direct C++ new practically does that; what is
>> missing currently is the locking when called via COM.
> 
> 	Sure - if they are running in the MTA, they can just wait for the
> SolarMutex - surely ?
> 
>> - we cannot assume that calls to the UNO event listener are made with
>> SolarMutex locked or not locked (maybe there are XAccessible impl.s that
>> don't use SolarMutex, or they release it prior to calling listeners?) -
>> it has to work for both possibilities
> 
> 	Really ? surely all XAccessible foo in practice is called with the
> SolarMutex held =) I'd be -amazed- if it was even safe to release it at
> that granularity ...
> 
>> so that suggests using SolarMutex.
> 
> 	Right.
> 
>> there is also the mysterious "if (!IsInMainThread())" check in
>> AccObjectWinManager::NotifyAccEvent() - not sure if that is supposed to
> 
> 	Yes - superstition is a wonderful thing =)
> 
>> so in summary it looks to me like putting SolarMutex guards in the
>> public CMAccessible methods is the way to go.
> 
> 	Agreed =)

ok, will do that some time...

>> and now for another fun problem i've seen: it's possible to deadlock
>> between the Win32 message handling in the main thread and deleting VCL
>> Window on other threads.  the ~Window destroys the native Win32 window
>> by sending a Win32 message to it, and the Win32 window apparently is
>> tied to a thread such that that message send blocks until the main
>> thread gets the message and acts on it.
> 
> 	Ok - I was also appalled by the void
> WinSalInstance::Yieldimplementation in the windows backend there too
> which is completely bogus. It basically assumes that the main thread (if
> not now running) will at some stage run - if we hang around a bit - and
> as such will be able to process a SAL_MSG_THREADYIELD message - which
> may well not work at all.
> 
> 	Then again - that's a bit intractable; we have to destroy windows on
> the thread there were created on (unfortunately), and several other
> types of operation have to be done there - there is about a dozen of
> them in vcl/win. Each of these mandates a ~synchronous call to the
> 'main' thread succeeding during them.
> 
>> so what can happen is that some other thread locks SolarMutex, deletes a
>> VCL Window, while main thread is in its message handling and tries to
>> lock SolarMutex itself, e.g. while getting a TopWindow via MSAA (but
>> there are many other messages which require locking SolarMutex); so this
>> is mostly a pre-existing problem and i'm not sure if anything can easily
>> be done about it; PostUserEvent apparently calls PostMessageW too so
>> likely has the same problem.
> 
> 	I guess it's pretty clear that we must never cause the main thread to
> stop executing while another thread wants to do something involving VCL.
> 
> 	Which is pretty much the same as saying that the Solar Mutex is the
> world's most appallingly bad joke. It makes you -think- you can do
> things on other threads (and under Unix/gtk+ you even can ;-) but when
> push comes to shove - if the main thread is not idling in the main-loop,
> ~nothing is going to work =)
> 
> 	What can we do about that ?
> 
> 	Well - we could special-case the SolarMutex on Windows - making it not
> a CriticalSection but a true Windows 'mutex' that we can include in a
> 'WaitForMultipleObjects' - along with the rest of the things the
> mainloop wants. Or we could associate a condition with it that can be
> signalled to wake the mainloop to ask for some event processing now and
> then - instead of just waiting on the lock/CriticalSection.

you mean some way to get the main loop to process outstanding Win32
messages while it fails to acquire SolarMutex?  ... sounds crazy ... but
what could possibly go wrong with that :)

> 	That might at least reduce the grief here of needing to get round-trips
> to a 'main' thread that is blocking on (effectively) us; that is a
> particularly silly deadlock ;-)
> 
> 	Also - I guess it'd be good to CC the developer list here (?) - I'm
> game to have my half-formed ramblings archived for posterity if you
> are ? ;-)

done