[PATCH 1/2] mfplat: Improve thread safety for IStream-based stream.
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/mfplat/main.c | 64 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 45 insertions(+), 19 deletions(-) diff --git a/dlls/mfplat/main.c b/dlls/mfplat/main.c index 13c48cfe23..14bde4a954 100644 --- a/dlls/mfplat/main.c +++ b/dlls/mfplat/main.c @@ -2730,11 +2730,18 @@ static HRESULT WINAPI bytestream_stream_SetLength(IMFByteStream *iface, QWORD le { struct bytestream *stream = impl_from_IMFByteStream(iface); ULARGE_INTEGER size; + HRESULT hr; TRACE("%p, %s.\n", iface, wine_dbgstr_longlong(length)); + EnterCriticalSection(&stream->cs); + size.QuadPart = length; - return IStream_SetSize(stream->stream, size); + hr = IStream_SetSize(stream->stream, size); + + LeaveCriticalSection(&stream->cs); + + return hr; } static HRESULT WINAPI bytestream_stream_GetCurrentPosition(IMFByteStream *iface, QWORD *position) @@ -2754,7 +2761,9 @@ static HRESULT WINAPI bytestream_stream_SetCurrentPosition(IMFByteStream *iface, TRACE("%p, %s.\n", iface, wine_dbgstr_longlong(position)); + EnterCriticalSection(&stream->cs); stream->position = position; + LeaveCriticalSection(&stream->cs); return S_OK; } @@ -2767,12 +2776,14 @@ static HRESULT WINAPI bytestream_stream_IsEndOfStream(IMFByteStream *iface, BOOL TRACE("%p, %p.\n", iface, ret); - if (FAILED(hr = IStream_Stat(stream->stream, &statstg, STATFLAG_NONAME))) - return hr; + EnterCriticalSection(&stream->cs); - *ret = stream->position >= statstg.cbSize.QuadPart; + if (SUCCEEDED(hr = IStream_Stat(stream->stream, &statstg, STATFLAG_NONAME))) + *ret = stream->position >= statstg.cbSize.QuadPart; - return S_OK; + LeaveCriticalSection(&stream->cs); + + return hr; } static HRESULT WINAPI bytestream_stream_Read(IMFByteStream *iface, BYTE *buffer, ULONG size, ULONG *read_len) @@ -2783,12 +2794,16 @@ static HRESULT WINAPI bytestream_stream_Read(IMFByteStream *iface, BYTE *buffer, TRACE("%p, %p, %u, %p.\n", iface, buffer, size, read_len); + EnterCriticalSection(&stream->cs); + position.QuadPart = stream->position; - if (FAILED(hr = IStream_Seek(stream->stream, position, STREAM_SEEK_SET, NULL))) - return hr; + if (SUCCEEDED(hr = IStream_Seek(stream->stream, position, STREAM_SEEK_SET, NULL))) + { + if (SUCCEEDED(hr = IStream_Read(stream->stream, buffer, size, read_len))) + stream->position += *read_len; + } - if (SUCCEEDED(hr = IStream_Read(stream->stream, buffer, size, read_len))) - stream->position += *read_len; + LeaveCriticalSection(&stream->cs); return hr; } @@ -2801,12 +2816,16 @@ static HRESULT WINAPI bytestream_stream_Write(IMFByteStream *iface, const BYTE * TRACE("%p, %p, %u, %p.\n", iface, buffer, size, written); + EnterCriticalSection(&stream->cs); + position.QuadPart = stream->position; - if (FAILED(hr = IStream_Seek(stream->stream, position, STREAM_SEEK_SET, NULL))) - return hr; + if (SUCCEEDED(hr = IStream_Seek(stream->stream, position, STREAM_SEEK_SET, NULL))) + { + if (SUCCEEDED(hr = IStream_Write(stream->stream, buffer, size, written))) + stream->position += *written; + } - if (SUCCEEDED(hr = IStream_Write(stream->stream, buffer, size, written))) - stream->position += *written; + LeaveCriticalSection(&stream->cs); return hr; } @@ -2815,9 +2834,12 @@ static HRESULT WINAPI bytestream_stream_Seek(IMFByteStream *iface, MFBYTESTREAM_ DWORD flags, QWORD *current) { struct bytestream *stream = impl_from_IMFByteStream(iface); + HRESULT hr = S_OK; TRACE("%p, %u, %s, %#x, %p.\n", iface, origin, wine_dbgstr_longlong(offset), flags, current); + EnterCriticalSection(&stream->cs); + switch (origin) { case msoBegin: @@ -2828,12 +2850,14 @@ static HRESULT WINAPI bytestream_stream_Seek(IMFByteStream *iface, MFBYTESTREAM_ break; default: WARN("Unknown origin mode %d.\n", origin); - return E_INVALIDARG; + hr = E_INVALIDARG; } *current = stream->position; - return S_OK; + LeaveCriticalSection(&stream->cs); + + return hr; } static HRESULT WINAPI bytestream_stream_Flush(IMFByteStream *iface) @@ -2948,6 +2972,8 @@ static HRESULT WINAPI bytestream_stream_read_callback_Invoke(IMFAsyncCallback *i op = impl_async_stream_op_from_IUnknown(object); + EnterCriticalSection(&stream->cs); + position.QuadPart = op->position; if (SUCCEEDED(hr = IStream_Seek(stream->stream, position, STREAM_SEEK_SET, NULL))) { @@ -2956,9 +2982,8 @@ static HRESULT WINAPI bytestream_stream_read_callback_Invoke(IMFAsyncCallback *i } IMFAsyncResult_SetStatus(op->caller, hr); - - EnterCriticalSection(&stream->cs); list_add_tail(&stream->pending, &op->entry); + LeaveCriticalSection(&stream->cs); MFInvokeCallback(op->caller); @@ -2979,6 +3004,8 @@ static HRESULT WINAPI bytestream_stream_write_callback_Invoke(IMFAsyncCallback * op = impl_async_stream_op_from_IUnknown(object); + EnterCriticalSection(&stream->cs); + position.QuadPart = op->position; if (SUCCEEDED(hr = IStream_Seek(stream->stream, position, STREAM_SEEK_SET, NULL))) { @@ -2987,9 +3014,8 @@ static HRESULT WINAPI bytestream_stream_write_callback_Invoke(IMFAsyncCallback * } IMFAsyncResult_SetStatus(op->caller, hr); - - EnterCriticalSection(&stream->cs); list_add_tail(&stream->pending, &op->entry); + LeaveCriticalSection(&stream->cs); MFInvokeCallback(op->caller); -- 2.20.1
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/mfplat/buffer.c | 4 ++-- dlls/mfplat/main.c | 35 ++++++++++++++++++++--------------- dlls/mfplat/mediatype.c | 12 ++++++------ dlls/mfplat/mfplat_private.h | 33 +++++++++++++++++++++++++++++++++ dlls/mfplat/queue.c | 1 + 5 files changed, 62 insertions(+), 23 deletions(-) diff --git a/dlls/mfplat/buffer.c b/dlls/mfplat/buffer.c index 27ae44a91c..0d912a7afe 100644 --- a/dlls/mfplat/buffer.c +++ b/dlls/mfplat/buffer.c @@ -311,7 +311,7 @@ static HRESULT WINAPI sample_CompareItem(IMFSample *iface, REFGUID key, REFPROPV { struct sample *sample = impl_from_IMFSample(iface); - TRACE("%p, %s, %p, %p.\n", iface, debugstr_attr(key), value, result); + TRACE("%p, %s, %s, %p.\n", iface, debugstr_attr(key), debugstr_propvar(value), result); return attributes_CompareItem(&sample->attributes, key, value, result); } @@ -429,7 +429,7 @@ static HRESULT WINAPI sample_SetItem(IMFSample *iface, REFGUID key, REFPROPVARIA { struct sample *sample = impl_from_IMFSample(iface); - TRACE("%p, %s, %p.\n", iface, debugstr_attr(key), value); + TRACE("%p, %s, %s.\n", iface, debugstr_attr(key), debugstr_propvar(value)); return attributes_SetItem(&sample->attributes, key, value); } diff --git a/dlls/mfplat/main.c b/dlls/mfplat/main.c index 14bde4a954..abb269c26f 100644 --- a/dlls/mfplat/main.c +++ b/dlls/mfplat/main.c @@ -1621,7 +1621,7 @@ static HRESULT WINAPI mfattributes_CompareItem(IMFAttributes *iface, REFGUID key { struct attributes *attributes = impl_from_IMFAttributes(iface); - TRACE("%p, %s, %p, %p.\n", iface, debugstr_attr(key), value, result); + TRACE("%p, %s, %s, %p.\n", iface, debugstr_attr(key), debugstr_propvar(value), result); return attributes_CompareItem(attributes, key, value, result); } @@ -1741,7 +1741,7 @@ static HRESULT WINAPI mfattributes_SetItem(IMFAttributes *iface, REFGUID key, RE { struct attributes *attributes = impl_from_IMFAttributes(iface); - TRACE("%p, %s, %p.\n", iface, debugstr_attr(key), value); + TRACE("%p, %s, %s.\n", iface, debugstr_attr(key), debugstr_propvar(value)); return attributes_SetItem(attributes, key, value); } @@ -3779,11 +3779,12 @@ static HRESULT WINAPI bytestream_wrapper_events_EndGetEvent(IMFMediaEventGenerat return IMFMediaEventGenerator_EndGetEvent(wrapper->event_generator, result, event); } -static HRESULT WINAPI bytestream_wrapper_events_QueueEvent(IMFMediaEventGenerator *iface, MediaEventType type, REFGUID ext_type, HRESULT hr, const PROPVARIANT *value) +static HRESULT WINAPI bytestream_wrapper_events_QueueEvent(IMFMediaEventGenerator *iface, MediaEventType type, + REFGUID ext_type, HRESULT hr, const PROPVARIANT *value) { struct bytestream_wrapper *wrapper = impl_wrapper_from_IMFMediaEventGenerator(iface); - TRACE("%p, %d, %s, %#x, %p.\n", iface, type, debugstr_guid(ext_type), hr, value); + TRACE("%p, %d, %s, %#x, %s.\n", iface, type, debugstr_guid(ext_type), hr, debugstr_propvar(value)); return IMFMediaEventGenerator_QueueEvent(wrapper->event_generator, type, ext_type, hr, value); } @@ -3900,11 +3901,12 @@ static HRESULT WINAPI bytestream_wrapper_propstore_GetValue(IPropertyStore *ifac return IPropertyStore_GetValue(wrapper->propstore, key, value); } -static HRESULT WINAPI bytestream_wrapper_propstore_SetValue(IPropertyStore *iface, REFPROPERTYKEY key, const PROPVARIANT *value) +static HRESULT WINAPI bytestream_wrapper_propstore_SetValue(IPropertyStore *iface, REFPROPERTYKEY key, + const PROPVARIANT *value) { struct bytestream_wrapper *wrapper = impl_wrapper_from_IPropertyStore(iface); - TRACE("%p, %p, %p.\n", iface, key, value); + TRACE("%p, %p, %s.\n", iface, key, debugstr_propvar(value)); return IPropertyStore_SetValue(wrapper->propstore, key, value); } @@ -3966,11 +3968,12 @@ static HRESULT WINAPI bytestream_wrapper_attributes_GetItemType(IMFAttributes *i return IMFAttributes_GetItemType(wrapper->attributes, key, type); } -static HRESULT WINAPI bytestream_wrapper_attributes_CompareItem(IMFAttributes *iface, REFGUID key, REFPROPVARIANT value, BOOL *result) +static HRESULT WINAPI bytestream_wrapper_attributes_CompareItem(IMFAttributes *iface, REFGUID key, + REFPROPVARIANT value, BOOL *result) { struct bytestream_wrapper *wrapper = impl_wrapper_from_IMFAttributes(iface); - TRACE("%p, %s, %p, %p.\n", iface, debugstr_attr(key), value, result); + TRACE("%p, %s, %s, %p.\n", iface, debugstr_attr(key), debugstr_propvar(value), result); return IMFAttributes_CompareItem(wrapper->attributes, key, value, result); } @@ -4090,7 +4093,7 @@ static HRESULT WINAPI bytestream_wrapper_attributes_SetItem(IMFAttributes *iface { struct bytestream_wrapper *wrapper = impl_wrapper_from_IMFAttributes(iface); - TRACE("%p, %s, %p.\n", iface, debugstr_attr(key), value); + TRACE("%p, %s, %s.\n", iface, debugstr_attr(key), debugstr_propvar(value)); return IMFAttributes_SetItem(wrapper->attributes, key, value); } @@ -5451,7 +5454,7 @@ static HRESULT WINAPI mfmediaevent_CompareItem(IMFMediaEvent *iface, REFGUID key { struct media_event *event = impl_from_IMFMediaEvent(iface); - TRACE("%p, %s, %p, %p.\n", iface, debugstr_attr(key), value, result); + TRACE("%p, %s, %s, %p.\n", iface, debugstr_attr(key), debugstr_propvar(value), result); return attributes_CompareItem(&event->attributes, key, value, result); } @@ -5572,7 +5575,7 @@ static HRESULT WINAPI mfmediaevent_SetItem(IMFMediaEvent *iface, REFGUID key, RE { struct media_event *event = impl_from_IMFMediaEvent(iface); - TRACE("%p, %s, %p.\n", iface, debugstr_attr(key), value); + TRACE("%p, %s, %s.\n", iface, debugstr_attr(key), debugstr_propvar(value)); return attributes_SetItem(&event->attributes, key, value); } @@ -5789,13 +5792,14 @@ static const IMFMediaEventVtbl mfmediaevent_vtbl = /*********************************************************************** * MFCreateMediaEvent (mfplat.@) */ -HRESULT WINAPI MFCreateMediaEvent(MediaEventType type, REFGUID extended_type, HRESULT status, - const PROPVARIANT *value, IMFMediaEvent **event) +HRESULT WINAPI MFCreateMediaEvent(MediaEventType type, REFGUID extended_type, HRESULT status, const PROPVARIANT *value, + IMFMediaEvent **event) { mfmediaevent *object; HRESULT hr; - TRACE("%s, %s, %08x, %p, %p\n", debugstr_eventid(type), debugstr_guid(extended_type), status, value, event); + TRACE("%s, %s, %08x, %p, %p\n", debugstr_eventid(type), debugstr_guid(extended_type), status, + debugstr_propvar(value), event); object = HeapAlloc( GetProcessHeap(), 0, sizeof(*object) ); if(!object) @@ -6069,7 +6073,8 @@ static HRESULT WINAPI eventqueue_QueueEventParamVar(IMFMediaEventQueue *iface, M IMFMediaEvent *event; HRESULT hr; - TRACE("%p, %s, %s, %#x, %p\n", iface, debugstr_eventid(event_type), debugstr_guid(extended_type), status, value); + TRACE("%p, %s, %s, %#x, %s\n", iface, debugstr_eventid(event_type), debugstr_guid(extended_type), status, + debugstr_propvar(value)); if (FAILED(hr = MFCreateMediaEvent(event_type, extended_type, status, value, &event))) return hr; diff --git a/dlls/mfplat/mediatype.c b/dlls/mfplat/mediatype.c index f4a2577c3a..aaf0714bdd 100644 --- a/dlls/mfplat/mediatype.c +++ b/dlls/mfplat/mediatype.c @@ -149,7 +149,7 @@ static HRESULT WINAPI mediatype_CompareItem(IMFMediaType *iface, REFGUID key, RE { struct media_type *media_type = impl_from_IMFMediaType(iface); - TRACE("%p, %s, %p, %p.\n", iface, debugstr_attr(key), value, result); + TRACE("%p, %s, %s, %p.\n", iface, debugstr_attr(key), debugstr_propvar(value), result); return attributes_CompareItem(&media_type->attributes, key, value, result); } @@ -270,7 +270,7 @@ static HRESULT WINAPI mediatype_SetItem(IMFMediaType *iface, REFGUID key, REFPRO { struct media_type *media_type = impl_from_IMFMediaType(iface); - TRACE("%p, %s, %p.\n", iface, debugstr_attr(key), value); + TRACE("%p, %s, %s.\n", iface, debugstr_attr(key), debugstr_propvar(value)); return attributes_SetItem(&media_type->attributes, key, value); } @@ -695,7 +695,7 @@ static HRESULT WINAPI stream_descriptor_CompareItem(IMFStreamDescriptor *iface, { struct stream_desc *stream_desc = impl_from_IMFStreamDescriptor(iface); - TRACE("%p, %s, %p, %p.\n", iface, debugstr_attr(key), value, result); + TRACE("%p, %s, %s, %p.\n", iface, debugstr_attr(key), debugstr_propvar(value), result); return attributes_CompareItem(&stream_desc->attributes, key, value, result); } @@ -817,7 +817,7 @@ static HRESULT WINAPI stream_descriptor_SetItem(IMFStreamDescriptor *iface, REFG { struct stream_desc *stream_desc = impl_from_IMFStreamDescriptor(iface); - TRACE("%p, %s, %p.\n", iface, debugstr_attr(key), value); + TRACE("%p, %s, %s.\n", iface, debugstr_attr(key), debugstr_propvar(value)); return attributes_SetItem(&stream_desc->attributes, key, value); } @@ -1271,7 +1271,7 @@ static HRESULT WINAPI presentation_descriptor_CompareItem(IMFPresentationDescrip { struct presentation_desc *presentation_desc = impl_from_IMFPresentationDescriptor(iface); - TRACE("%p, %s, %p, %p.\n", iface, debugstr_attr(key), value, result); + TRACE("%p, %s, %s, %p.\n", iface, debugstr_attr(key), debugstr_propvar(value), result); return attributes_CompareItem(&presentation_desc->attributes, key, value, result); } @@ -1396,7 +1396,7 @@ static HRESULT WINAPI presentation_descriptor_SetItem(IMFPresentationDescriptor { struct presentation_desc *presentation_desc = impl_from_IMFPresentationDescriptor(iface); - TRACE("%p, %s, %p.\n", iface, debugstr_attr(key), value); + TRACE("%p, %s, %s.\n", iface, debugstr_attr(key), debugstr_propvar(value)); return attributes_SetItem(&presentation_desc->attributes, key, value); } diff --git a/dlls/mfplat/mfplat_private.h b/dlls/mfplat/mfplat_private.h index d7a7547f03..57e4211e40 100644 --- a/dlls/mfplat/mfplat_private.h +++ b/dlls/mfplat/mfplat_private.h @@ -16,6 +16,8 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */ +#define NONAMELESSUNION + #undef INITGUID #include <guiddef.h> #include "mfapi.h" @@ -23,6 +25,7 @@ #include "mferror.h" #include "wine/heap.h" +#include "wine/debug.h" struct attribute { @@ -114,3 +117,33 @@ static inline BOOL mf_array_reserve(void **elements, size_t *capacity, size_t co return TRUE; } + +static inline const char *debugstr_propvar(const PROPVARIANT *v) +{ + if (!v) + return "(null)"; + + switch (v->vt) + { + case VT_EMPTY: + return wine_dbg_sprintf("%p {VT_EMPTY}", v); + case VT_NULL: + return wine_dbg_sprintf("%p {VT_NULL}", v); + case VT_UI4: + return wine_dbg_sprintf("%p {VT_UI4: %d}", v, v->u.ulVal); + case VT_UI8: + return wine_dbg_sprintf("%p {VT_UI8: %s}", v, wine_dbgstr_longlong(v->u.uhVal.QuadPart)); + case VT_R8: + return wine_dbg_sprintf("%p {VT_R8: %lf}", v, v->u.dblVal); + case VT_CLSID: + return wine_dbg_sprintf("%p {VT_CLSID: %s}", v, debugstr_guid(v->u.puuid)); + case VT_LPWSTR: + return wine_dbg_sprintf("%p {VT_LPWSTR: %s}", v, wine_dbgstr_w(v->u.pwszVal)); + case VT_VECTOR | VT_UI1: + return wine_dbg_sprintf("%p {VT_VECTOR|VT_UI1: %p}", v, v->u.caub.pElems); + case VT_UNKNOWN: + return wine_dbg_sprintf("%p {VT_UNKNOWN: %p}", v, v->u.punkVal); + default: + return wine_dbg_sprintf("%p {vt %#x}", v, v->vt); + } +} diff --git a/dlls/mfplat/queue.c b/dlls/mfplat/queue.c index 36c50566d3..31b281a9a3 100644 --- a/dlls/mfplat/queue.c +++ b/dlls/mfplat/queue.c @@ -19,6 +19,7 @@ #include <stdarg.h> #define COBJMACROS +#define NONAMELESSUNION #include "wine/debug.h" #include "wine/list.h" -- 2.20.1
Hi, While running your changed tests, 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=51587 Your paranoid android. === debian9 (32 bit report) === mfplat: mfplat.c:2482: Test failed: Unexpected refcount 1. Unhandled exception: page fault on execute access to 0x2f2e2e2f in 32-bit code (0x2f2e2e2f). Report errors: mfplat:mfplat crashed (c0000005)
participants (2)
-
Marvin -
Nikolay Sivov