Conversation
Try
left a comment
Try
left a comment
There was a problem hiding this comment.
Hi @Solessfir!
I think it would be right to adjust code of dmusic rather. Also wonder, how did you hit this case? Did mtx_lock failed for you?
| DmResult rv = DmLoader_create(&dmLoader, DmLoader_DOWNLOAD); | ||
| if(rv != DmResult_SUCCESS) { | ||
| Log::e("Failed to created DmLoader object. Out of memory?"); | ||
| dmLoader = nullptr; |
There was a problem hiding this comment.
I'm pretty sure, It's not intended for library code (DmLoader_create) to change &dmLoader on error.
So, correct way to fix it is to PR into DirectMusic.
|
|
||
| DmLoader_addResolver(dmLoader, [](void* ctx, char const* name, size_t* len) -> void* { | ||
| else { | ||
| DmLoader_addResolver(dmLoader, [](void* ctx, char const* name, size_t* len) -> void* { |
There was a problem hiding this comment.
also not need - DmLoader_* meant to early out and return DmResult_INVALID_ARGUMENT, for null-argument.
|
Makes sense. I've prepared and committed the fix in dmusic locally: leave the output pointer untouched on failure and assign it only after initialization succeeds. - DmLoader* new = *slf = Dm_alloc(sizeof *new);
+ DmLoader* new = Dm_alloc(sizeof *new);Then add You're right about the resolver guard too: once the loader pointer stays null, I found this while investigating an Android crash when quitting Gothic 1. The startup log reported loader creation failure, but didn't record the result code. The dangling-pointer path I found is the |
|
Opened the dmusic fix here: GothicKit/dmusic#14. It assigns the output pointer only after initialization succeeds. This replaces the caller-side workaround proposed here once OpenGothic updates its dmusic submodule. |
|
I'll close this one for now - will update dmusic, once PR is merged |
2 participants
When DirectMusic loader initialization fails,
DmLoader_createcan free its allocation while leaving the output pointer non-null. OpenGothic then passes that dangling pointer toDmLoader_addResolverand laterDmLoader_release.Clear the pointer on failure and register the resolver only after successful initialization. This prevents use-after-free during startup and shutdown.
Found while investigating a Gothic 1 crash on Quit: loader initialization failed, and shutdown crashed while destroying the VFS tree. The change eliminated that native crash in the Android build. It affects shared resource initialization and requires no Android code.