Skip to content

Commit 233f211

Browse files
James Chenminggo
James Chen
authored and
minggo
committed
fixed #16170: Random crash in alGenBuffers(AudioCache::readDataTask) at startup. (#16182)
* fixed #16170: Random crash in alGenBuffers(AudioCache::readDataTask) at startup. * Minor fix: should -> may * Minor fix: updates comments. * Update comment. * Comment fix again.
1 parent bb25bed commit 233f211

File tree

1 file changed

+63
-0
lines changed

1 file changed

+63
-0
lines changed

cocos/audio/apple/AudioEngine-inl.mm

+63
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,69 @@ -(void) dealloc
192192
for (int i = 0; i < MAX_AUDIOINSTANCES; ++i) {
193193
_alSourceUsed[_alSources[i]] = false;
194194
}
195+
196+
// fixed #16170: Random crash in alGenBuffers(AudioCache::readDataTask) at startup
197+
// Please note that, as we know the OpenAL operation is atomic (threadsafe),
198+
// 'alGenBuffers' may be invoked by different threads. But in current implementation of 'alGenBuffers',
199+
// When the first time it's invoked, application may crash!!!
200+
// Why? OpenAL is opensource by Apple and could be found at
201+
// http://opensource.apple.com/source/OpenAL/OpenAL-48.7/Source/OpenAL/oalImp.cpp .
202+
/*
203+
204+
void InitializeBufferMap()
205+
{
206+
if (gOALBufferMap == NULL) // Position 1
207+
{
208+
gOALBufferMap = new OALBufferMap (); // Position 2
209+
210+
// Position Gap
211+
212+
gBufferMapLock = new CAGuard("OAL:BufferMapLock"); // Position 3
213+
gDeadOALBufferMap = new OALBufferMap ();
214+
215+
OALBuffer *newBuffer = new OALBuffer (AL_NONE);
216+
gOALBufferMap->Add(AL_NONE, &newBuffer);
217+
}
218+
}
219+
220+
AL_API ALvoid AL_APIENTRY alGenBuffers(ALsizei n, ALuint *bids)
221+
{
222+
...
223+
224+
try {
225+
if (n < 0)
226+
throw ((OSStatus) AL_INVALID_VALUE);
227+
228+
InitializeBufferMap();
229+
if (gOALBufferMap == NULL)
230+
throw ((OSStatus) AL_INVALID_OPERATION);
231+
232+
CAGuard::Locker locked(*gBufferMapLock); // Position 4
233+
...
234+
...
235+
}
236+
237+
*/
238+
// 'gBufferMapLock' will be initialized in the 'InitializeBufferMap' function,
239+
// that's the problem. It means that 'InitializeBufferMap' may be invoked in different threads.
240+
// It will be very dangerous in multi-threads environment.
241+
// Imagine there're two threads (Thread A, Thread B), they call 'alGenBuffers' simultaneously.
242+
// While A goto 'Position Gap', 'gOALBufferMap' was assigned, then B goto 'Position 1' and find
243+
// that 'gOALBufferMap' isn't NULL, B just jump over 'InitialBufferMap' and goto 'Position 4'.
244+
// Meanwhile, A is still at 'Position Gap', B will crash at '*gBufferMapLock' since 'gBufferMapLock'
245+
// is still a null pointer. Oops, how could Apple implemented this method in this fucking way?
246+
247+
// Workaround is do an unused invocation in the mainthread right after OpenAL is initialized successfully
248+
// as bellow.
249+
// ================ Workaround begin ================ //
250+
251+
ALuint unusedAlBufferId = 0;
252+
alGenBuffers(1, &unusedAlBufferId);
253+
alDeleteBuffers(1, &unusedAlBufferId);
254+
255+
// ================ Workaround end ================ //
256+
257+
195258
_scheduler = Director::getInstance()->getScheduler();
196259
ret = true;
197260
ALOGI("OpenAL was initialized successfully!");

0 commit comments

Comments
 (0)