actor list access violation from new sound

[Florian]

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
[Switching to process 3159 thread 0x497f]
update_sound (ms=20) at sound.c:3992
3992					if (actors_list[i]->actor_id == yourself)
(gdb) bt full
#0  update_sound (ms=20) at sound.c:3992
	i = 18
	pSource = (source_data *) 0x5da94f0
	pSample = (sound_sample *) 0x0
	pSoundType = (sound_type *) 0x17a93400
	pVariant = (sound_variants *) 0x4264e48
	deadBuffer = 2415937195
	numProcessed = -1339625896
	buffer = -1339625824
	state = -1339625880
	source = 3
	distanceSq = 2
	maxDistSq = 344907988
	relative = -1879030101
	tx = 101
	ty = 141
	sourcePos = {123.518517, 135, 0}
	listenerPos = {101, 141, 0}
	listenerVel = {0, 0, 0}
	listenerOri = {-1, 1, 0, 0, 0, 1}

 

/EDIT

this was at them beam btw, so I guess the actor list gets changed a lot there.

Edited November 28, 2007 by Florian

[alvieboy]

We should look at all actor list accesses and properly lock the beast. Or all stuff that sound thread touches.

 

Anyone wants to do it or shall I ?

[bluap]

We should look at all actor list accesses and properly lock the beast. Or all stuff that sound thread touches.

Anyone wants to do it or shall I ?

This was suggested during testing. There's only one place, in update_sound() the might need the mutex lock. Torg was going to investigate but I guess he ran out of time. I have only ever had one crash in that bit of code and spend ages fighting on the test server to try reproduce it. I also added the mutex to my code and played for a while, the addition was harmless.

[alvieboy]

There's only one place, in update_sound() the might need the mutex lock.

 

You need to lock *all* accesses, not only in update_sound(). Otherwise it's futile.

 

Maybe better with RW locks though. It's way faster, if the sound thread iterates it often.

[bluap]

There's only one place, in update_sound() the might need the mutex lock.

 

You need to lock *all* accesses, not only in update_sound(). Otherwise it's futile.

Agreed, locking all access is the safest way. However, in this case, if you are a read only accessor in the same thread as the only writer then it's not going to be needed. Only read access for threads other than the writer is dangerous. It does need checking out though. I hope I'm making sense, its been a long day. Coincidently, I've been implementing mutex locking code at work today.

[belton]

only update_sound looks into actors_list

otherwise, uses of your_actor should be replaced with get_our_actor, but anything else? (whenever a sound.c function got an actor pointer, the lock should already be held)

just adding a lock doesn't work for deadlock reasons (with stop_sound, which is called from destroy_actors, with actors lock held, then taking the sound lock).

maybe there should be a simple order: actors, then sound. the actors lock be taken by the sound updating timer

 

with a locking order, the fix to update_sound doesn't hang

It seems like there are alot of accesses to your_actor and to the actor returned by get_our_actor, that do not hold the actors lock?