"Maarten Lankhorst" m.b.lankhorst@gmail.com writes:
diff --git a/dlls/quartz/acmwrapper.c b/dlls/quartz/acmwrapper.c index d877930..22bfb87 100644 --- a/dlls/quartz/acmwrapper.c +++ b/dlls/quartz/acmwrapper.c @@ -186,7 +186,9 @@ static HRESULT ACMWrapper_ProcessSampleData(TransformFilterImpl* pTransformFilte } TRACE("Sample stop time: %u.%03u\n", (DWORD)(tStart/10000000), (DWORD)((tStart/10000)%1000));
LeaveCriticalSection(&This->tf.csFilter); hr = OutputPin_SendSample((OutputPin*)This->tf.ppPins[1], pOutSample);EnterCriticalSection(&This->tf.csFilter);
This looks very wrong this the lock wasn't grabbed by this function, it will violate expectations of the caller. It looks like the locking strategy needs some more thought.
Hi Alexandre,
2008/7/7 Alexandre Julliard julliard@winehq.org:
"Maarten Lankhorst" m.b.lankhorst@gmail.com writes:
diff --git a/dlls/quartz/acmwrapper.c b/dlls/quartz/acmwrapper.c index d877930..22bfb87 100644 --- a/dlls/quartz/acmwrapper.c +++ b/dlls/quartz/acmwrapper.c @@ -186,7 +186,9 @@ static HRESULT ACMWrapper_ProcessSampleData(TransformFilterImpl* pTransformFilte } TRACE("Sample stop time: %u.%03u\n", (DWORD)(tStart/10000000), (DWORD)((tStart/10000)%1000));
LeaveCriticalSection(&This->tf.csFilter); hr = OutputPin_SendSample((OutputPin*)This->tf.ppPins[1], pOutSample);EnterCriticalSection(&This->tf.csFilter);This looks very wrong this the lock wasn't grabbed by this function, it will violate expectations of the caller. It looks like the locking strategy needs some more thought.
If you look at It won't violate the locking rules, since this is the way it is designed. If you look at OutputPin_SendSample, you will see it takes the critical section to obtain whether the pin is connected to something, and if it is, it will send the data further. It does a great job of doing it safely, by obtaining all data under lock and adding a reference so that it can't die while receiving. The data is received by the receiving pin which takes a lock while processing the data, so the code is thread-safe.
Cheers, Maarten.
"Maarten Lankhorst" m.b.lankhorst@gmail.com writes:
It won't violate the locking rules, since this is the way it is designed. If you look at OutputPin_SendSample, you will see it takes the critical section to obtain whether the pin is connected to something, and if it is, it will send the data further. It does a great job of doing it safely, by obtaining all data under lock and adding a reference so that it can't die while receiving. The data is received by the receiving pin which takes a lock while processing the data, so the code is thread-safe.
I'm not saying it's not thread-safe, I'm saying it's wrong to release a lock obtained by a caller. When you read the caller's code and you see an Enter/Leave pair you expect that the lock is held all along, you can't have inferior functions release it. The code needs to be restructured so that Enter/Leave are properly matched.
-
Alexandre Julliard -
Maarten Lankhorst