-Codechange: move GameLoop_Main() into a thread #206

Closed
wants to merge 3 commits into
from

Projects

None yet

5 participants

@glx22
Member
glx22 commented Apr 7, 2013

Another approach to try to fix the deadlock issue.

@rofl0r
rofl0r commented Apr 7, 2013

how is that supposed to help ?
the video display functions access global objects, which are also accessed from the game logic functions.
accessing those vars is inherently non-thread safe unless you impose a global lock or shield each of the vars with a separate mutex.

@TrueBrain
Member

@miniupnp : does this work on OSX?

To all: does this fix Linux issues?

(basically: bump :D )

@miniupnp
Collaborator

@TrueBrain on OS X it doesn't get Mouse/Keyboard events
and output a lot of
2013-04-17 19:30:18.158 opendune[927] *** _NSAutoreleaseNoPool(): Object 0x6ab420 of class NSCFArray autoreleased with no pool in place - just leaking 2013-04-17 19:30:18.160 opendune[927] *** _NSAutoreleaseNoPool(): Object 0x6161b0 of class NSWindowGraphicsContext autoreleased with no pool in place - just leaking
errors.

@TrueBrain
Member

I don't mean this the bad way, but are you sure you are testing a clean version of this PR? The errors are a bit weird for what this patch does ... silly OSX :(

@miniupnp
Collaborator

I'll test again, but it's quite simple : just don't call any Cocoa code (SDL does) outside the main thread if you want to keep out of trouble :)

@TrueBrain
Member

This is why your findings are weird: it all happens in the main thread. At least, that was the idea :)

@glx22
Member
glx22 commented Apr 17, 2013

It just works in my 10.6.2 VM (but I had no deadlocks in this VM nor in the debian VM)

@miniupnp
Collaborator

I think there are SDL calls in GameLoop_Main(). At least some palette
change calls.
Anyway as you already said, the real solution is changing the code so we
have a real main loop which handles GUI Events, paints, etc.
without any additional thread or signal...

@glx22
Member
glx22 commented Apr 17, 2013

Indeed Video_SetPalette() can be the cause, it's called at the begining of GameLoop_Main(), and many other times from callees of this loop, especially on palette animation (GUI_PaletteAnimate()). I guess it should be possible to delegate Video_SetPalette() to the main thread via some kind of communication between the threads.

@miniupnp
Collaborator

It works on my OS X 10.6.8 but not on 10.4.11...

@TrueBrain
Member

How about we ifdef OSX 10.4 (and 10.5?) to use signals, and switch the rest to threads? It is a dirty solution, but at least it makes a workable version for most of the platforms with MIDI enabled.

Good idea / bad idea?

@wangds
Collaborator
wangds commented Apr 21, 2013

This doesn't work very well for me in Linux. Switching between the main game and the F2 menu usually crashes. Video_SetPalette seems to be the cause. So, bad idea.

@TrueBrain
Member

Ah, that is feedback we need. I didnt know it didn't work on linux. But I believe there was a solution for that problem .. @glx22: didnt you had a patch for it?

@TrueBrain
Member

Maybe I should stress this a bit more: both me and glx22 cannot reproduce it on any of our machines, the problems with signals and threads etc. So we really depend on your feedback to tell us if it works now yes/no ;)

@miniupnp miniupnp closed this Jun 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment