Locking bug?

[revi]

I think there's a potential bug with locking in actors.c, line 1600:

 

if(i==-1) return;//A nasty error occured and we couldn't add the actor. Ignore it.

	//The actors list is locked when we get here...

if you end up there wit i==-1, you return without unlocking the actors list.

 

I started looking at this in relation with a crash when standing next to the invasion master while he was filling the map (and boy, did he fill it...).

The crash happened a short while after the filling, when I was again able to walk out of the densest area. "Segment violation", no stack traces or core dump available, though. Given the situation, I suspect a null pointer dereference in combination with a race condition. Still looking at potential candidates for that.

[bluap]

It's certainly a bug and caused a freeze on exit for me when I triggered the condition.  I've committed a change to unlock the actors list.  It's interesting code....

[revi]

"Interesting" is one way to describe it

 

Threre are quite a few accesses to the actors_list[] not protected by any lock.

Is that on purpose, as in the lock isn't needed, since there's no possibility that the

list is changed (or used) by another thread? In at least one case, the routine involved

does change items in actors_list[]

 

[bluap]

Unfortunately, I'm not that familiar with that code, most of it was written before I even started playing.  I've explored it a few times with the same  thoughts you have and noticed the lack of locks in some places.  Normally, if you have locks for a resource, you'd protect all access including reads.  Read-only locks do not have to be exclusive, but a write lock should be exclusive and prevent reads too.

Edited October 25, 2020 by bluap