Race condition: running_check, performance concern

[argoneuscze]

Here:

tsuserver3/server/tsuserver.py

Lines 134 to 146 in b1bddd6

	def running_check(self, runner):
	self.runtime += 1
	if self.runtime % 60 == 0:
	for area in self.area_manager.areas:
	print(area.last_talked)
	area.last_talked = None
	self.stats_manager.save_alldata()
	Timer(1, self.running_check, [runner]).start()
	def load_runner(self):
	self.runner = threading.Event()
	self.running_check(self.runner)

A thread changes area.last_talked, which is later changed here, but in a separate thread.

tsuserver3/server/aoprotocol.py

Lines 401 to 404 in b1bddd6

	if not self.client.area.basement:
	if self.client.area.last_talked is None:
	self.client.area.last_talked = self.client.ipid
	self.server.stats_manager.char_talked(self.client.char_id, self.client.ipid, self.client.area.status)

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:

tsuserver3/server/tsuserver.py

Lines 144 to 145 in 32657d5

	async def load_runner(self):
	asyncio.ensure_future(self.running_check())

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:

tsuserver3/server/tsuserver.py

Line 98 in 32657d5

loop.run_until_complete(self.load_runner())

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