MegaBits / SIOSocket

Realtime iOS application framework (client) http://socket.io
MIT License
494 stars 80 forks source link

Ensure that all block callbacks are on Main Thread #28

Closed jonathannorris closed 9 years ago

jonathannorris commented 9 years ago

I believe that the majority of issues that people have been seeing with instability on iOS 8 (specifically #16) are because most of the callback blocks are running on a WebThread and not the Main Thread. You can see this by adding a log for the current thread in one of the return blocks:

socket.javascriptContext[@"objc_onConnect"] = ^() {
    NSLog(@"javascript thread: %@", [NSThread currentThread]);

    dispatch_async(dispatch_get_main_queue(), ^{
        NSLog(@"main thread: %@", [NSThread currentThread]);
        if (weakSocket.onConnect)
            weakSocket.onConnect();
    });
};

Obviously all callbacks should be called from the main thread.

iliu commented 9 years ago

Hey @jonathannorris , although this seems correct, i think it could still introduce subtle threading issues depending on what's run in the blocks. For example, if you called socket emit within the callback block, it would try to run javascriptContext evaluateScript on the main thread (since dispatch_async around socket emit is commented out in your branch), which would cause the same threading issues that was fixed before.

I think the main fix of the threading issue is that we need to ensure all javascriptcontext evaluateScript occurs on one thread.

jonathannorris commented 9 years ago

@iliu totally agree, I am going to try and get this fixed today.

I've tried to make all javascriptcontext evaluateScript calls happen on a new thread, but that seemed to have its own threading issues. Really all calls to javascriptcontext evaluateScript should be happening on its own Web Thread, but I'm not sure how to get that thread to ensure that the on:callback: and emit:args: calls are on that Web Thread...

iliu commented 9 years ago

this pull request is a start - https://github.com/MegaBits/SIOSocket/pull/42

jonathannorris commented 9 years ago

closing this pull request in favor of #45