[PATCH 0/5] MR9408: dmloader: Reduce memory usage part 3
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/dmloader/tests/loader.c | 49 ++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/dlls/dmloader/tests/loader.c b/dlls/dmloader/tests/loader.c index 3d68a19c331..8c8f613575b 100644 --- a/dlls/dmloader/tests/loader.c +++ b/dlls/dmloader/tests/loader.c @@ -112,7 +112,11 @@ static void test_directory(void) static void test_caching(void) { + IDirectMusicCollection *collection; IDirectMusicLoader8 *loader = NULL; + IDirectMusicSegment *segment; + DMUS_OBJECTDESC desc; + ULONG refcount; HRESULT hr; hr = CoCreateInstance(&CLSID_DirectMusicLoader, NULL, CLSCTX_INPROC, &IID_IDirectMusicLoader8, @@ -145,6 +149,51 @@ static void test_caching(void) hr = IDirectMusicLoader_EnableCache(loader, &CLSID_DirectMusicContainer, TRUE); ok(hr == S_FALSE, "EnableCache failed with %#lx\n", hr); + memset(&desc, 0, sizeof(desc)); + desc.dwSize = sizeof(desc); + desc.dwValidData = DMUS_OBJ_OBJECT | DMUS_OBJ_CLASS; + desc.guidObject = GUID_DefaultGMCollection; + desc.guidClass = CLSID_DirectMusicCollection; + hr = IDirectMusicLoader_GetObject(loader, &desc, &IID_IDirectMusicCollection, (void **)&collection); + ok(hr == S_OK, "GetObject failed with %#lx\n", hr); + IDirectMusicCollection_AddRef(collection); + refcount = IDirectMusicCollection_Release(collection); + ok(refcount == 2, "refcount == %lu, expected 2\n", refcount); + + memset(&desc, 0, sizeof(desc)); + desc.dwSize = sizeof(desc); + desc.dwValidData = DMUS_OBJ_CLASS | DMUS_OBJ_MEMORY; + desc.guidClass = CLSID_DirectMusicSegment; + desc.llMemLength = sizeof(rifffile); + desc.pbMemData = rifffile; + hr = IDirectMusicLoader_GetObject(loader, &desc, &IID_IDirectMusicSegment, (void **)&segment); + ok(hr == S_OK, "GetObject failed with %#lx\n", hr); + IDirectMusicSegment_AddRef(segment); + refcount = IDirectMusicSegment_Release(segment); + ok(refcount == 2, "refcount == %lu, expected 2\n", refcount); + + hr = IDirectMusicLoader_ClearCache(loader, &GUID_DirectMusicAllTypes); + ok(hr == S_OK, "ClearCache failed with %#lx\n", hr); + + /* ClearCache releases the loaded objects */ + IDirectMusicCollection_AddRef(collection); + refcount = IDirectMusicCollection_Release(collection); + todo_wine ok(refcount == 1, "refcount == %lu, expected 1\n", refcount); + IDirectMusicSegment_AddRef(segment); + refcount = IDirectMusicSegment_Release(segment); + todo_wine ok(refcount == 1, "refcount == %lu, expected 1\n", refcount); + + /* ClearCache doesn't remove the default collection */ + memset(&desc, 0, sizeof(desc)); + desc.dwSize = sizeof(desc); + hr = IDirectMusicLoader_EnumObject(loader, &CLSID_DirectMusicCollection, 0, &desc); + ok(hr == S_OK, "EnumObject failed with %#lx\n", hr); + ok(IsEqualGUID(&desc.guidObject, &GUID_DefaultGMCollection), + "Got object guid %s, expected GUID_DefaultGMCollection\n", + wine_dbgstr_guid(&desc.guidObject)); + + IDirectMusicCollection_Release(collection); + IDirectMusicSegment_Release(segment); IDirectMusicLoader_Release(loader); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9408
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/dmloader/loader.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dlls/dmloader/loader.c b/dlls/dmloader/loader.c index 3798a0851a0..966d6fa1b80 100644 --- a/dlls/dmloader/loader.c +++ b/dlls/dmloader/loader.c @@ -169,9 +169,15 @@ static ULONG WINAPI loader_Release(IDirectMusicLoader8 *iface) TRACE("(%p)->(): new ref = %lu\n", iface, ref); if (!ref) { + struct cache_entry *obj, *obj2; unsigned int i; - IDirectMusicLoader8_ClearCache(iface, &GUID_DirectMusicAllTypes); + LIST_FOR_EACH_ENTRY_SAFE(obj, obj2, &This->cache, struct cache_entry, entry) { + if ((obj->Desc.dwValidData & DMUS_OBJ_LOADED) && obj->pObject) + IDirectMusicObject_Release(obj->pObject); + list_remove(&obj->entry); + free(obj); + } for (i = 0; i < ARRAY_SIZE(classes); i++) free(This->search_paths[i]); free(This); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9408
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/dmloader/loader.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/dmloader/loader.c b/dlls/dmloader/loader.c index 966d6fa1b80..bfcc32da49c 100644 --- a/dlls/dmloader/loader.c +++ b/dlls/dmloader/loader.c @@ -749,8 +749,10 @@ static HRESULT WINAPI loader_ClearCache(IDirectMusicLoader8 *iface, REFGUID clas (obj->Desc.dwValidData & DMUS_OBJ_LOADED)) { /* basically, wrap to ReleaseObject for each object found */ IDirectMusicLoader8_ReleaseObject(iface, obj->pObject); - list_remove(&obj->entry); - free(obj); + if (!IsEqualGUID(&GUID_DefaultGMCollection, &obj->Desc.guidObject)) { + list_remove(&obj->entry); + free(obj); + } } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9408
From: Anton Baskanov <baskanov(a)gmail.com> Otherwise, the object won't be found in the cache and will leak on release. --- dlls/dmloader/loader.c | 2 ++ dlls/dmloader/tests/loader.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/dlls/dmloader/loader.c b/dlls/dmloader/loader.c index bfcc32da49c..d8bad56c7f2 100644 --- a/dlls/dmloader/loader.c +++ b/dlls/dmloader/loader.c @@ -445,12 +445,14 @@ static HRESULT WINAPI loader_GetObject(IDirectMusicLoader8 *iface, DMUS_OBJECTDE pObjectEntry = calloc(1, sizeof(*pObjectEntry)); DM_STRUCT_INIT(&pObjectEntry->Desc); DMUSIC_CopyDescriptor (&pObjectEntry->Desc, &GotDesc); + pObjectEntry->Desc.dwValidData |= DMUS_OBJ_LOADED; pObjectEntry->pObject = pObject; list_add_head(&This->cache, &pObjectEntry->entry); } else { DMUSIC_CopyDescriptor (&pObjectEntry->Desc, &GotDesc); + pObjectEntry->Desc.dwValidData |= DMUS_OBJ_LOADED; pObjectEntry->pObject = pObject; } TRACE(": filled in cache entry\n"); diff --git a/dlls/dmloader/tests/loader.c b/dlls/dmloader/tests/loader.c index 8c8f613575b..65946f3d04e 100644 --- a/dlls/dmloader/tests/loader.c +++ b/dlls/dmloader/tests/loader.c @@ -178,7 +178,7 @@ static void test_caching(void) /* ClearCache releases the loaded objects */ IDirectMusicCollection_AddRef(collection); refcount = IDirectMusicCollection_Release(collection); - todo_wine ok(refcount == 1, "refcount == %lu, expected 1\n", refcount); + ok(refcount == 1, "refcount == %lu, expected 1\n", refcount); IDirectMusicSegment_AddRef(segment); refcount = IDirectMusicSegment_Release(segment); todo_wine ok(refcount == 1, "refcount == %lu, expected 1\n", refcount); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9408
From: Anton Baskanov <baskanov(a)gmail.com> Some objects have no GUID or name (e.g. segments created from MIDI files), so ReleaseObject() won't find them in the cache. --- dlls/dmloader/loader.c | 7 +++++-- dlls/dmloader/tests/loader.c | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/dlls/dmloader/loader.c b/dlls/dmloader/loader.c index d8bad56c7f2..0291fffc06d 100644 --- a/dlls/dmloader/loader.c +++ b/dlls/dmloader/loader.c @@ -749,8 +749,11 @@ static HRESULT WINAPI loader_ClearCache(IDirectMusicLoader8 *iface, REFGUID clas LIST_FOR_EACH_ENTRY_SAFE(obj, obj2, &This->cache, struct cache_entry, entry) { if ((IsEqualGUID(class, &GUID_DirectMusicAllTypes) || IsEqualGUID(class, &obj->Desc.guidClass)) && (obj->Desc.dwValidData & DMUS_OBJ_LOADED)) { - /* basically, wrap to ReleaseObject for each object found */ - IDirectMusicLoader8_ReleaseObject(iface, obj->pObject); + if (obj->pObject) { + IDirectMusicObject_Release(obj->pObject); + obj->pObject = NULL; + } + obj->Desc.dwValidData &= ~DMUS_OBJ_LOADED; if (!IsEqualGUID(&GUID_DefaultGMCollection, &obj->Desc.guidObject)) { list_remove(&obj->entry); free(obj); diff --git a/dlls/dmloader/tests/loader.c b/dlls/dmloader/tests/loader.c index 65946f3d04e..d318a7b155a 100644 --- a/dlls/dmloader/tests/loader.c +++ b/dlls/dmloader/tests/loader.c @@ -181,7 +181,7 @@ static void test_caching(void) ok(refcount == 1, "refcount == %lu, expected 1\n", refcount); IDirectMusicSegment_AddRef(segment); refcount = IDirectMusicSegment_Release(segment); - todo_wine ok(refcount == 1, "refcount == %lu, expected 1\n", refcount); + ok(refcount == 1, "refcount == %lu, expected 1\n", refcount); /* ClearCache doesn't remove the default collection */ memset(&desc, 0, sizeof(desc)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9408
@rbernon In [!4339 (comment 51683)](https://gitlab.winehq.org/wine/wine/-/merge_requests/4339#note_51683) you mentioned that d20f430c breaks Final Fantasy VIII in-game music. Do you remember what was the issue exactly? I tried to enter and exit locations with different music, and it seems to play fine (tested the Stream version and the demo). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9408#note_121279
Michael Stefaniuc (@mstefani) commented about dlls/dmloader/tests/loader.c:
+ desc.guidObject = GUID_DefaultGMCollection; + desc.guidClass = CLSID_DirectMusicCollection; + hr = IDirectMusicLoader_GetObject(loader, &desc, &IID_IDirectMusicCollection, (void **)&collection); + ok(hr == S_OK, "GetObject failed with %#lx\n", hr); + IDirectMusicCollection_AddRef(collection); + refcount = IDirectMusicCollection_Release(collection); + ok(refcount == 2, "refcount == %lu, expected 2\n", refcount); + + memset(&desc, 0, sizeof(desc)); + desc.dwSize = sizeof(desc); + desc.dwValidData = DMUS_OBJ_CLASS | DMUS_OBJ_MEMORY; + desc.guidClass = CLSID_DirectMusicSegment; + desc.llMemLength = sizeof(rifffile); + desc.pbMemData = rifffile; + hr = IDirectMusicLoader_GetObject(loader, &desc, &IID_IDirectMusicSegment, (void **)&segment); + ok(hr == S_OK, "GetObject failed with %#lx\n", hr); This fails the Win10 64bit tests and then subsequently crashes:
https://gitlab.winehq.org/wine/wine/-/jobs/205418#L72 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9408#note_121283
participants (3)
-
Anton Baskanov -
Anton Baskanov (＠baskanov)
-
Michael Stefaniuc (＠mstefani)