Race in FACTAudioEngine_Initialize

j-r commented

2021-01-08 09:37:40 -06:00

(Migrated from github.com)

On my (old and slow) system Hyperdimension Neptunia ReBirth1 exits silently (probably catching the crash) pretty soon after initializing (see +xact3 log in linked wine bug).

This problem vanishes when single stepping through FACTAudioEngine_Initialize or randomly sprinkling some FAudio_sleep(1000) into it.

Have no idea where to look now.

On my (old and slow) system Hyperdimension Neptunia ReBirth1 exits silently (probably catching the crash) pretty soon after initializing (see +xact3 log in linked wine bug). This problem vanishes when single stepping through FACTAudioEngine_Initialize or randomly sprinkling some FAudio_sleep(1000) into it. Have no idea where to look now.

flibitijibibo commented

2021-01-08 10:35:47 -06:00

(Migrated from github.com)

Here's where the thread is made: https://github.com/FNA-XNA/FAudio/blob/master/src/FACT.c#L254

The backing FAudio thread should be fine, FACT would surprise me though since there's a massive mutex protecting everything.

Here's where the thread is made: https://github.com/FNA-XNA/FAudio/blob/master/src/FACT.c#L254 The backing FAudio thread should be fine, FACT would surprise me though since there's a massive mutex protecting everything.

j-r commented

2021-01-08 12:49:31 -06:00

(Migrated from github.com)

Thanks! It seems that one sleep between CreateMasteringVoice and PlatformCreateThread is needed. Perhaps the device needs some time to settle down?

flibitijibibo commented

2021-01-08 14:51:10 -06:00

(Migrated from github.com)

Does it still die if you put the sleep after submix creation? FAudio's create functions pretty much all block so it surprises me that it causes any issues to begin with... for reference, the thread is stuck waiting on the mutex that the main thread holds until everything's started:

https://github.com/FNA-XNA/FAudio/blob/master/src/FACT_internal.c#L1640

Does it still die if you put the sleep after submix creation? FAudio's create functions pretty much all block so it surprises me that it causes any issues to begin with... for reference, the thread is stuck waiting on the mutex that the main thread holds until everything's started: https://github.com/FNA-XNA/FAudio/blob/master/src/FACT_internal.c#L1640

flibitijibibo commented

2021-01-08 15:01:02 -06:00

(Migrated from github.com)

On the subject of threads I do think I've found something: If the game registers global callbacks and the API thread executes them, we may be running into a situation where the native thread isn't compatible with the Wine thread. This is one reason we had EngineProcedureEXT:

https://github.com/FNA-XNA/FAudio/blob/master/extensions/EngineProcedureEXT.txt

We may need to amend this extension to include an export for the FACT engine thread. If these calls from the log...

0024:trace:xact3:IXACT3EngineImpl_RegisterNotification (0x17c6e8)->(0x31f784)
0024:trace:xact3:unwrap_notificationdesc Type 17
0024:trace:xact3:IXACT3EngineImpl_RegisterNotification (0x17c6e8)->(0x31f784)
0024:trace:xact3:unwrap_notificationdesc Type 6
0024:trace:xact3:IXACT3EngineImpl_RegisterNotification (0x17c6e8)->(0x31f784)
0024:trace:xact3:unwrap_notificationdesc Type 3
0024:trace:xact3:IXACT3EngineImpl_RegisterNotification (0x17c6e8)->(0x31f784)
0024:trace:xact3:unwrap_notificationdesc Type 1

... have structs where these four guys are always NULL...

https://github.com/FNA-XNA/FAudio/blob/master/include/FACT.h#L290-L293

... that's exactly what we're looking at. Even if this isn't the case for this exact game it probably should get checked out, so CC'ing @aeikum since he wrote the previous extension.

On the subject of threads I do think I've found something: If the game registers global callbacks and the API thread executes them, we may be running into a situation where the native thread isn't compatible with the Wine thread. This is one reason we had EngineProcedureEXT: https://github.com/FNA-XNA/FAudio/blob/master/extensions/EngineProcedureEXT.txt We may need to amend this extension to include an export for the FACT engine thread. If these calls from the log... ``` 0024:trace:xact3:IXACT3EngineImpl_RegisterNotification (0x17c6e8)->(0x31f784) 0024:trace:xact3:unwrap_notificationdesc Type 17 0024:trace:xact3:IXACT3EngineImpl_RegisterNotification (0x17c6e8)->(0x31f784) 0024:trace:xact3:unwrap_notificationdesc Type 6 0024:trace:xact3:IXACT3EngineImpl_RegisterNotification (0x17c6e8)->(0x31f784) 0024:trace:xact3:unwrap_notificationdesc Type 3 0024:trace:xact3:IXACT3EngineImpl_RegisterNotification (0x17c6e8)->(0x31f784) 0024:trace:xact3:unwrap_notificationdesc Type 1 ``` ... have structs where these four guys are always NULL... https://github.com/FNA-XNA/FAudio/blob/master/include/FACT.h#L290-L293 ... that's exactly what we're looking at. Even if this isn't the case for this exact game it probably should get checked out, so CC'ing @aeikum since he wrote the previous extension.

j-r commented

2021-01-10 05:35:36 -06:00

(Migrated from github.com)

Thanks for the pointers. The problem is the thread priority call at

https://github.com/FNA-XNA/FAudio/blob/0f3f1e6df74da481d466dd97aa4345ea9fe56ca4/src/FACT_internal.c#L1651

If it is commented out, the problem vanishes. I played around a little more with the sleep and it has to be after

https://github.com/FNA-XNA/FAudio/blob/0f3f1e6df74da481d466dd97aa4345ea9fe56ca4/src/FAudio_platform_sdl2.c#L153

Makes it look like an internal SDL problem. It might be an instance of https://bugzilla.libsdl.org/show_bug.cgi?id=5093.

Thanks for the pointers. The problem is the thread priority call at [https://github.com/FNA-XNA/FAudio/blob/0f3f1e6df74da481d466dd97aa4345ea9fe56ca4/src/FACT_internal.c#L1651](https://github.com/FNA-XNA/FAudio/blob/0f3f1e6df74da481d466dd97aa4345ea9fe56ca4/src/FACT_internal.c#L1651) If it is commented out, the problem vanishes. I played around a little more with the sleep and it has to be after [https://github.com/FNA-XNA/FAudio/blob/0f3f1e6df74da481d466dd97aa4345ea9fe56ca4/src/FAudio_platform_sdl2.c#L153](https://github.com/FNA-XNA/FAudio/blob/0f3f1e6df74da481d466dd97aa4345ea9fe56ca4/src/FAudio_platform_sdl2.c#L153) Makes it look like an internal SDL problem. It might be an instance of [https://bugzilla.libsdl.org/show_bug.cgi?id=5093](https://bugzilla.libsdl.org/show_bug.cgi?id=5093).

flibitijibibo commented

2021-01-10 05:47:20 -06:00

(Migrated from github.com)

Does the game produce multiple sound devices? If not, it's likely we're looking at something else... but having thread priority affect the outcome reaffirms the existence of a race somewhere.

j-r commented

2021-01-10 06:26:13 -06:00

(Migrated from github.com)

Does the game produce multiple sound devices? If not, it's likely we're looking at something else... but having thread priority affect the outcome reaffirms the existence of a race somewhere.

My resasoning is like this: we know that the problem (which is of yet not fully diagnosed, but possibly a crash - perhaps inside SDL - that I fail to observe accurately, because I'm not smart enough to look at the right place:-) is triggered by calling SDL_SetPriority very shortly after SDL_OpenAudioDevice. I don't think the actual thread priority matters at all (because the problem also gets hidden by some random amount of waiting befor SDL_SetPriority), but looking at the linked bug it mentions that the crash happens because of a race in the internal thread priority manipulation code path. Here we have AFAICT only a single audio device, but also a seperate call to SDL_SetPriority that might exercise the same path.

I have no clear idea how to verify that, though. So perhaps there are other more promising tests to do. I'm open to suggestions.

(Unfortunately my holiday is ending today, but daycare facilities will remain closed over here, so probably I won't have time to work further on this for at least some weeks.)

> Does the game produce multiple sound devices? If not, it's likely we're looking at something else... but having thread priority affect the outcome reaffirms the existence of a race somewhere. My resasoning is like this: we know that the problem (which is of yet not fully diagnosed, but possibly a crash - perhaps inside SDL - that I fail to observe accurately, because I'm not smart enough to look at the right place:-) is triggered by calling SDL_SetPriority very shortly after SDL_OpenAudioDevice. I don't think the actual thread priority matters at all (because the problem also gets hidden by some random amount of waiting befor SDL_SetPriority), but looking at the linked bug it mentions that the crash happens because of a race in the internal thread priority manipulation code path. Here we have AFAICT only a single audio device, but also a seperate call to SDL_SetPriority that might exercise the same path. I have no clear idea how to verify that, though. So perhaps there are other more promising tests to do. I'm open to suggestions. (Unfortunately my holiday is ending today, but daycare facilities will remain closed over here, so probably I won't have time to work further on this for at least some weeks.)

alesliehughes commented

2021-01-25 01:34:21 -06:00

(Migrated from github.com)

Some applications create multiple xaudio devies, which inturn calls SDL_OpenAudioDevice multiple times. There was an issue, when SDL failed to open the device, using pulse, it would attempt to teardown the device, leading to a crash.

SDL 2.0.14 has a fix for the above scenario.

With respect to the notification, wine-staging has a proof of concept patch for the XACTNOTIFICATIONTYPE_WAVEBANKPREPARED. This patch can be applied directly on top of wine if you want to give it a go.

https://github.com/wine-staging/wine-staging/tree/master/patches/xactengine3_7-Notification

Some applications create multiple xaudio devies, which inturn calls SDL_OpenAudioDevice multiple times. There was an issue, when SDL failed to open the device, using pulse, it would attempt to teardown the device, leading to a crash. SDL 2.0.14 has a fix for the above scenario. With respect to the notification, wine-staging has a proof of concept patch for the XACTNOTIFICATIONTYPE_WAVEBANKPREPARED. This patch can be applied directly on top of wine if you want to give it a go. https://github.com/wine-staging/wine-staging/tree/master/patches/xactengine3_7-Notification

orbea commented

2021-03-01 20:48:34 -06:00

(Migrated from github.com)

@flibitijibibo I am not sure if this is the same as this issue, but would you mind looking at this wine issue?

https://bugs.winehq.org/show_bug.cgi?id=50757

It only crashes/freezes when wine is built against FAudio and seems related to to the referenced wine issue (https://bugs.winehq.org/show_bug.cgi?id=50416).

I also bisected it to this wine commit. github.com/wine-mirror/wine@127ef80f0f

@flibitijibibo I am not sure if this is the same as this issue, but would you mind looking at this wine issue? https://bugs.winehq.org/show_bug.cgi?id=50757 It only crashes/freezes when wine is built against FAudio and seems related to to the referenced wine issue (https://bugs.winehq.org/show_bug.cgi?id=50416). I also bisected it to this wine commit. https://github.com/wine-mirror/wine/commit/127ef80f0f9370a148357b7c1f001c3477a52264

flibitijibibo commented

2021-03-01 21:46:11 -06:00

(Migrated from github.com)

It's not us, FNA uses this API and its callbacks with no issue - you may be looking at native<->Wine thread corruption issues. It is indeed a separate issue based on the bisect.

👍 1

flibitijibibo commented

2021-03-05 15:47:40 -06:00

(Migrated from github.com)