Socket.io-client-swift: Multiple data races found by Xcode Thread Sanitizer

[rehsals]

Hi!
As per this WWDC session Thread Sanitizer in Xcode helps to find data races.
I use socket.io client in my project and just by running it with limited use cases found some data races.
Maybe they are nothing much, but I think it could be useful to run full socket.io test-suite with Thread Sanitizer enabled to make sure everything is fine. Maybe it will help to eliminate misterious crashes here and there.
What do you think?
Thanks and best regards.

[nuclearace]

Can thread sanitizer be on during a running app? The unit tests probably won't detect the more critical problems, since they aren't doing any tests _with_ socket.io. At one point we had integration tests, but they don't work well with travis. Might look into having them just be manually run tests.

[rehsals]

I think thread sanitizer works only with simulator but it can be on during a running app. That was the way I found out about mentioned data races.

[kikolobo]

Found same issues here... I am getting lots of thread sanitizer warnings:

This is most likely because something is called from different threads.

My warnings occur around the public func doPoll() { in the: SocketEnginePollable.swift file

heres the extended info from LLDB debugger:

"stop_description" : "Data race detected",
  "summary" : "Data race in SocketIO.SocketEngine.waitingForPoll.getter : Swift.Bool at 0x7d4400000b40",
  "threads" : [
    {
      "index" : 0,
      "name" : "",
      "parent_thread_id" : 0,
      "running" : 1,
      "thread_id" : 12,
      "thread_os_id" : 498251,
      "trace" : [

      ]
    },
    {
      "index" : 1,
      "name" : "",
      "parent_thread_id" : 17,
      "running" : 1,
      "thread_id" : 17,
      "thread_os_id" : 38912,
      "trace" : [

      ]
    }
  ],
  "unique_tids" : [

[kikolobo]

This is really annoying and hard to debug. I simplified the socket and still get this error randomly but constantly. I am targeting iOS9 and using iPad simulator. Also I am using Swift 3.

[kikolobo]

@rehsals

I think thread sanitizer works only with simulator but it can be on during a running app. That was the way I found out about mentioned data races.

Yes it can be on while running the app. (that's the whole point of the sanitizer) You have to enable the threadSanitizer in the scheme. It will not be available if you are compiling with legacy swift 2.x .. Only pure objective-c and/or Swift 3.

It will show on runtime errors sub tab in the messages/error tab in xCode.

Once enable connect to a server and send some traffic to see the threadSanitizer fire. This is an important issue that affects the stability of all the app if it is not fixed soon.

ping: @nuclearace

[kikolobo]

@nuclearace It seems the issue is at polling. If I use .forceWebsockets(true) option on connect the race condition only occurs when trying to reconnect again if for some reason I disconnect. Or when pinging/ponging:

[nuclearace]

Hm, yeah, polling is pretty ugly. I'm probably missing a dispatch somewhere in the polling code. Around this code perhaps? https://github.com/socketio/socket.io-client-swift/blob/master/Source/SocketEnginePollable.swift#L139

[kikolobo]

@nuclearace Il take a look and try to test.. Thanks for the comment.

[kikolobo]

@nuclearace Not on that line.. I tried it and it didn't fixed it. I am searching elsewhere but I don't have a lot of context of the code at this point. Hope we can solve this soon. It randomly produces crashes.
Specially on reconnects.

[kikolobo]

Question: Why are you dispatching to all those queues? Why not only dispatch on callbacks or closures? All that threading sounds hard to keep track what happens where and the performance gains might be killed by the thread problems.

[nuclearace]

The idea was that the engine and client could be separated, so each has their own handle, parse, and emit queues. And it's more to do with async than performance, I don't want to block the main queue while I do a bunch of parsing and such. Plus I use dispatch queues to try and limit access to resources. The trouble with polling is that I don't think I properly put back the response on a correct queue. Right now it looks like URLSession will call https://github.com/socketio/socket.io-client-swift/blob/master/Source/SocketEnginePollable.swift#L116 this callback on the main queue, which is wrong. Trouble is URLSession is OperationQueue based, not DispatchQueue based.

[nuclearace]

Re-use of the engine is something I don't really like, and was added later. I'd rather throw the engine away after a disconnect. Which is what .forceNew does.

[kikolobo]

Re-using engine did not fixed race condition. Hope this can get fixed soon.

I am suspecting one fix could be to use only one queue for dispatch and that that queue is not the main thread.

[kikolobo]

@nuclearace Hi.. Just checking.. No new developments here? This have potential to crash the whole app... Would be important to fix to be able to use the code in production ready app.

[nuclearace]

I haven't had time to look into it.

[kikolobo]

😕