OGRECave / ogre-audiovideo

plugins for theora video playback and openAL audio
https://ogrecave.github.io/ogre-audiovideo/
BSD 3-Clause "New" or "Revised" License
14 stars 11 forks source link

OgreOggSound crashes when exiting and threading is enabled #16

Closed sercero closed 3 years ago

sercero commented 3 years ago

Hello @paroj, I'm trying now to solve another Segmentation fault.

This also happens when exiting an application that is using OgreOggSound, but this time when threading is enabled.

I don't have a complete backtrace, but stepping through the debugger the culprit is this line:

    OgreOggSoundManager::~OgreOggSoundManager()
    {
#if OGGSOUND_THREADED
        mShuttingDown = true;

        if ( mUpdateThread )
        {
            mUpdateThread->join();  <--- !!! HERE !!!
            OGRE_FREE(mUpdateThread, Ogre::MEMCATEGORY_GENERAL);
            mUpdateThread = 0;
            mShuttingDown = false;

It seems that when calling for the thread to join it crashes with the following incomplete backtrace:

Thread 24 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 18952.0x5130]
0x000000006782cce8 in soft_oal!alDistanceModel () from D:\OGRE2\Built\openal-soft-1.20.1-bin\bin\Win64\soft_oal.dll
(gdb) bt
#0  0x000000006782cce8 in soft_oal!alDistanceModel () from D:\OGRE2\Built\openal-soft-1.20.1-bin\bin\Win64\soft_oal.dll
#1  0x00000000678aa559 in soft_oal!alcCaptureOpenDevice () from D:\OGRE2\Built\openal-soft-1.20.1-bin\bin\Win64\soft_oal.dll
#2  0x0000000067894ba1 in soft_oal!alcCaptureOpenDevice () from D:\OGRE2\Built\openal-soft-1.20.1-bin\bin\Win64\soft_oal.dll
#3  0x0000000067894e0a in soft_oal!alcCaptureOpenDevice () from D:\OGRE2\Built\openal-soft-1.20.1-bin\bin\Win64\soft_oal.dll
#4  0x00007fffbf849a1d in ntdll!RtlActivateActivationContextUnsafeFast () from C:\WINDOWS\SYSTEM32\ntdll.dll
#5  0x00007fffbf849aff in ntdll!RtlActivateActivationContextUnsafeFast () from C:\WINDOWS\SYSTEM32\ntdll.dll
#6  0x00007fffbf8475a3 in ntdll!LdrShutdownThread () from C:\WINDOWS\SYSTEM32\ntdll.dll
#7  0x00007fffbf88462e in ntdll!RtlExitUserThread () from C:\WINDOWS\SYSTEM32\ntdll.dll
#8  0x00007fffbe4fafa7 in msvcrt!_endthreadex () from C:\WINDOWS\System32\msvcrt.dll
#9  0x0000000064944b19 in pthread_create_wrapper () from D:\OGRE2\CodeBlocks\MinGW\opt\bin\libwinpthread-1.dll
#10 0x00007fffbe4faf5a in msvcrt!_beginthreadex () from C:\WINDOWS\System32\msvcrt.dll
#11 0x00007fffbe4fb02c in msvcrt!_endthreadex () from C:\WINDOWS\System32\msvcrt.dll
#12 0x00007fffbe417034 in KERNEL32!BaseThreadInitThunk () from C:\WINDOWS\System32\kernel32.dll
#13 0x00007fffbf882651 in ntdll!RtlUserThreadStart () from C:\WINDOWS\SYSTEM32\ntdll.dll
#14 0x0000000000000000 in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt stack?)

I have tried naively to add a lock on OgreOggSoundManager::getSingletonPtr()->mMutex or have the thread wait for 10ms after setting mShuttingDown = true without success.

Do you have any ideas?

Thanks!

paroj commented 3 years ago

are you using the code from your PR? If so, there are some non _WQ lock variants, which are noops. This might lead to wrong method call order i.e. use of a deleted object.

sercero commented 3 years ago

No, I am using code from another PR which I didn't submit (option 1).

I will see what happens with the code from the PR you saw and check the non _WQ lock variants.

Thanks!

sercero commented 3 years ago

Hey @paroj, I replaced all the OGRE_LOCK_MUTEX_NAMED for OGRE_WQ_LOCK_MUTEX_NAMED.

There is no change, in fact I see this in OgreThreadDefines.h:

#define OGRE_LOCK_MUTEX_NAMED(mutexName, lockName) OGRE_WQ_LOCK_MUTEX_NAMED(mutexName, lockName)

And those are all the locks I'm using.

There seems to be a problem with the usage of the replacements in OgreThreadDefines.h, because when using the code from the PR#14 the crash is even worse.

So, I have two branches:

When using branch std_thread the application crashes on exit, but only when using the debugger to step through the code. If using release or debug normally the crash does not happen (there is still something wrong) but the user won't notice.

When using branch std_thread2 the crash happens always and it is much more visible.

If you want I can submit another PR with branch std_thread so you can check that out.

Also let me know if you think that this is too much work for minimal gain 😭

paroj commented 3 years ago

There is no change, in fact I see this in OgreThreadDefines.h:

this is only used if OGRE_THREAD_SUPPORT != 3, but 3 is the default (only workqueue syncronised).

When using branch std_thread2 the crash happens always and it is much more visible.

when it comes to threading a reproducible crash is better than a crash that only happens sometimes. With a debugger attached, the application execution is slowed down, so that crash might also happen in release mode on a slower computer.

I will try to take a look at this soon.

sercero commented 3 years ago

when it comes to threading a reproducible crash is better than a crash that only happens sometimes. With a debugger attached, the application execution is slowed down, so that crash might also happen in release mode on a slower computer.

When I said always what I meant was that it happened both in release and debug version with and without the debugger attached, it is a much harder crash.

The other crash (the one referenced in this thread) is also very consistent and happens always when exiting and joining the other thread but it happens only when you are running the debug version inside a debugger which is a situation that a user is very unlikely to come up with.

If you want I can submit another patch request from the other branch so you can see what I'm talking about.

Anyway, I really apreciate you looking into this because this problem goes over my head.

Thanks!

sercero commented 3 years ago

It seems that the problem was related to OpenAL Soft for Windows in some way, I am now using the latest version 1.21.1 and the problem is gone.

paroj commented 3 years ago

also giving you maintainer access, as you know the code better than me by now