Race condition: running_check, performance concern

[argoneuscze]

Here: https://github.com/AttorneyOnlineVidya/tsuserver3/blob/b1bddd6db528fab9356ebec6f3f07f4fca146fc3/server/tsuserver.py#L134-L146 A thread changes area.last_talked, which is later changed here, but in a separate thread. https://github.com/AttorneyOnlineVidya/tsuserver3/blob/b1bddd6db528fab9356ebec6f3f07f4fca146fc3/server/aoprotocol.py#L401-L404

The timer, if anything, should work asynchronously sharing the same asyncio event loop as the protocol code. This way there is a race condition where you are reading/writing a value in two separate threads.

Furthermore, you're creating a new Timer every second instead of reusing one, which is going to be slow, as there is a new thread spawned and killed every second.

Also, the runner variable doesn't seem to do anything. What is the point of it?

I also suggest removing the timer entirely and creating a solution that compares timestamps instead of a timer that runs every second.

[argoneuscze]

Also as a note, you're writing three different files every second in the same function with self.stats_manager.save_alldata(). At that point you might want to switch to an actual database, but that's out of the scope of this issue.

EDIT: Correction, it saves the files every minute, still.

[oldmud0]

Remark: GIL prevents serious concurrency violations from occurring, but I see what your point is - threading is not necessary to solve this problem.

[argoneuscze]

Relying on the GIL is ugly to say the least. The GIL is not something that's specified somewhere, it's an implementation detail of CPython - what if they're using another implementation? What if someone decides to port the server to Cython eventually? For example Jython or IronPython don't even have a concept of GIL.

[oldmud0]

You can still have a concurrency violation even with the GIL. Two coroutines concurrently modifying a complex shared resource may result in unintended behavior.

[argoneuscze]

Looks a bit better now, but there's still some things:

https://github.com/AttorneyOnlineVidya/tsuserver3/blob/32657d5dce03bb0aeda5e745b8bc9dc2647ff5e6/server/tsuserver.py#L144-L145 This function doesn't need to be async - all it does is create (and return) a new task in the event loop, which isn't an asynchronous action.

And if that one is not asynchronous, this line is redundant: https://github.com/AttorneyOnlineVidya/tsuserver3/blob/32657d5dce03bb0aeda5e745b8bc9dc2647ff5e6/server/tsuserver.py#L98

That's all for the asynchronous part, as far as I can see.