FYI Re: wined3d_mutex

[Max Woodbury]

On 03/17/2014 10:50 AM, Stefan Dösinger wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Am 2014-03-17 15:30, schrieb Max TenEyck Woodbury:
>> Well, at least as fine as native.  Finer might be safer since there
>> is little besides direct examination of the native execution stream
>> (a big NONO!) that assures that all grain boundaries have been
>> identified.
>
> An application may (inadvertently rather than knowingly) expect d3d to
> block the execution of one of its threads and corrupt internal data if
> we don't. Unless we don't know of any such case we don't have to copy
> every stupid behavior native has, but there's also no point in being
> extra clever about things when applications don't expect d3d to do that.
>
Yes, but it still might be 'better to err on the side of caution'...

>> OK.  I believe you, but exactly what kind of lock is it and how did
>> you go about identifying it?  The fact that 'we' hold it during the
>> call backs is something that bothered me.  I also noted that 'we'
>> sometimes take it out twice which also makes me think its use is
>> rather superficial.
>
> Something like this:
>
> event e1 = cleared, e2 = cleared;
> IDirectDraw ddraw1, ddraw2;
>
> Thread 1:
> {
>      Start thread 2;
>      Wait for event e1;
>      ddraw1->AddRef();
>      Signal event e2;
>      return;
> }
>
> Thread 2:
> {
>      ddraw2->EnumSurfaces (callback);
>      callback:
>      {
>          Signal event e1;
>          Wait for event e2 - expect a timeout;
>      }
> }
>
> The Addref call in thread 1 does not finish until the EnumSurfaces
> callback finishes. Since the callback waits for e2 this pseudocode
> deadlocks. The deadlock is detected by a timed out wait with a
> reasonably large (e.g. 500ms) wait time.
>
> This deadlock even happens with two totally unrelated ddraw objects,
> and functions that shouldn't need a lock like AddRef. (AddRef can just
> be implemented using interlocked increment ops like in Wine.)
>
> Alexandre said there was no point in having such a test in the tree
> unless we find an application that depends on the coarse locking
> behavior. You can search wine-patches for the test, I think I
> submitted it somewhen in late 2006 or 2007.
>
> The same can be done in d3d9, but the AddRef and Release methods of an
> IUnknown * passed to SetPrivateData are the only case I know of where
> those libraries call application code. I expect d3d9 to be better wrt
> concurrency than ddraw. But it needs tests, not suspicions.
>
OK. that shows that there is such a lock, but it leaves the question of
exactly what kind of lock it is open.  If I remember correctly, there
are three kinds:

- Critical sections that can be entered multiple times by a thread
without blocking but delays other threads, which is what wine uses.

- Mutex locks that would block if the same thread tries to take it out
a second time.

- Read/write locks that can be taken out many times by many threads for 
reading, but only allow one writer which blocks further readers and
waits for existing readers to finish.

max