GarageGames / Torque3D

MIT Licensed Open Source version of Torque 3D from GarageGames
http://torque3d.org
MIT License
3.36k stars 1.2k forks source link

Console Logger is not Thread Safe #390

Open jamesu opened 11 years ago

jamesu commented 11 years ago

I bumped into an issue with the console logger in a project. If we have the following situation:

Thread 1: Con::printf("One"); Thread 2: Con::printf("Two"); Thread 3: Con::printf("Three");

Usually you'd expect the following to be sent through to the consumers, in whatever order the threads manage to call the consumer:

One
Two
Three

Depending on the timing of these threads you may get the following sent to the consumers:

One
Three

In console.cpp we have:

static void _printf(ConsoleLogEntry::Level level, ConsoleLogEntry::Type type, const char* fmt, va_list argptr)
{
   if (!active)
       return;
   Con::active = false; 

Essentially the "active" check is just a check to prevent a recursion later on in the code. What is happening is since there is no synchronization in _printf, Thread 1 may set active to true causing Thread 2 to ignore the print, while Thread 3 may find active is false again as Thread 1 has finished printing the entry.

If you have the log buffer enabled or the console, you'll likely get a crash as yet again there is no synchronization whatsoever when pushing the log entry to the console log.

In my case since I don't use any of the console stuff (just the consumer list) I solved the issue by just having a lock on the consumer, but I figured maybe a better solution is needed for Torque3D...

just-bank commented 11 years ago

Some time ago I've posted this resource on GG: http://www.garagegames.com/community/resources/view/21238 [Thread-safe console for Torque 3D] May be similar to this can be done here?

crabmusket commented 10 years ago

@just-bank those changes look like they'll solve the issue for now. But I'd like to discuss ways in which we could really make T3D multithread-friendly. Globally locking the whole string table is probably not a great way to do this. I imagine it would result in high contention :P.

just-bank commented 10 years ago

@eightyeight we use it in AfterWorld and it works well without any hit on performance. Mentioned resource is only a small part of the multithreading we have done in our game, lets call it "basics", so it is at least possible to do Con::printf(). In our game we can even register SimObjects in a separate thread and do a lot of other stuff. I'll discuss this with the team if we can find a way of moving parts of it to the T3D MIT.