What is wrong with my @synchronized block?

Question

I have 2 threads in my application, a game update thread and render/IO/main thread. My update thread updates the game state, and the render thread renders the scene based on the updated values of the game state models and a few other variables stored inside an object (gameEngine).

The render thread gets executed while the game thread is still updating, which is a problem, so it appeared to me the solution is to use @synchronized like this:

        @synchronized(gameEngine)
        {
            [gameEngine update];

            nextUpdate = now + GAME_UPDATE_INTERVAL;

            gameEngine.lastGameUpdateInterval = now - lastUpdate;
            gameEngine.lastGameUpdateTime = now;
            lastUpdate = now;
        }

But the render thread still accesses the gameEngine object between -update and the last 3 lines of the block. Why is this?

Answer 1

@synchronized does not block other threads from accessing gameEngine. It just blocks other @synchronized with the same object. That means in

// thread A:
@synchronized(a) {
   do_A(a);
}
...
// thread B:
do_B(a);

do_A and do_B can happen together, but in

// thread A:
@synchronized(a) {
   do_A(a);
}
...
// thread B:
@syncrhonized(a) {
   do_B(a);
}

do_A and do_B will always be executed sequentially.

Answer 2

You shouldn't be locking the gameEngine -- @KennyTM explained what I believe is the correct answer to your question, however implementing it would lead to only either the game engine or the renderer being able to execute at a given time, making it essentially single threaded. What you should be doing is using an immutable state object, which may be a ivar of gameEngine, but should be nonatomic, where in the render function you grab the state like so

State *state = [[gameEngine state] retain];

and then use state, releasing it when done. When the gameEngine performs an update, it should not mutate the data in its state ivar, which might be being used by the renderer, but may make a copy of it. To set the state, it should

State *oldState = state;
state = newState; //newState should have a retainCount of 1
[oldState release];

because if you release oldState before setting state to newState then the renderer might get oldState, which was just deallocated, leading to Bad Things.