getnamo / SocketIOClient-Unreal

Socket.IO client plugin for the Unreal Engine.
Other
872 stars 238 forks source link

Hard crash in 4.24.3 and 4.26.1 (calling connect twice) #256

Open IRTCKyleR opened 3 years ago

IRTCKyleR commented 3 years ago

Windows reports the crash as a stack buffer overrun, obviously very helpful image

The problem is somewhat intermittent, however it's about a 50/50 shot that on play in editor it will crash, it is slightly better behaved in a packaged build. image

Further back in the call stack we eventually get to this point: image

Which is in the client_impl::connect function.

This happened on 4.24.3 so I moved forward to 4.26.1 to see if that would fix the problem, however it did not resolve the issue.

I am statically constructing a component as in the picture below: image

And I am connecting afterward as shown: image

The general flow of my application is as follows: the program starts up, the intro level loads, depending on some configuration variables it opens a different level, the socketio connection gets created as part of the gamestate on the server and then connects leading to the above.

The closest thing I've been able to find to this problem is the following: https://github.com/Icinga/icinga2/issues/7431 Where a security scan caused issues in the boost asio library which is also present here.

I've been using the plugin for about a year now and it has worked very well, however these hard crashes have become more frequent. Unfortunately I can't control the environment I develop in(which combined with the asio library was the ultimate cause for the problem in the link above), so I'm hoping there is a simple solution to the problem available.

IRTCKyleR commented 3 years ago

Update: I think I've figured out what the actual root of the problem is.

I was trying to figure out why I was hitting connection code before I called the connect node from blueprint so I set some breakpoints to find out that the SocketIONative::Connect function is being called for some reason. To explore this, I set four breakpoints.

The first is in the socketionative::connect function: image

The second is in the client_impl::connect function: image

The third is in the client_impl::connect_impl function: image

and the fourth is on my connect node in blueprint: image

What seems to be happening is the native connect function is called (probably due to auto-connect which I tried to disable), which sends us into client_impl::connect which executes connect_impl using asio.

THEN we hit the breakpoint I set in blueprint on the connect node, and I was trying to figure this out, when the connect_impl breakpoint got hit again after a couple of seconds of waiting at the connect node.

I figured out that if I wait at this breakpoint in blueprint and let the second connect_impl call happen, I can avoid the crash.

I think what's happening is a race condition, where one of the connection procedures (either the ongoing auto or my explicit) is trying to delete a thread that no longer exists because of the other. Hence the error originating here: image

EDIT:

Solved, I was calling connect before where I thought I was, and thus was calling it twice in the same frame. However, it does seem like there should be some safety for multiple connection calls.

getnamo commented 3 years ago

Thanks for the very detailed debug. I agree with the conclusion, we need to add some safety to ensure it doesn't crash silently in a race condition. To add data to debug, what's the context (blueprint owner) of where the construct static component is called? Gameinstance subclass? Also do you use plugin scoping on your connection or not?

IRTCKyleR commented 3 years ago

No problem, the issue was hanging up development since it would crash the editor so high priority on my side.

As far as the context goes, I have kind of a weird system that's necessary because of the number of projects I work on.

In this case, the server instance of the GameState spawns MyActorComponent at runtime (very similarly to how you spawn the SocketIO component statically) so it's owner is the gamestate. Then the server instance of MyActorComponent spawns the SocketIOComponent before calling connect.

I don't use plugin scoped connections as I have some cleanup logic tied to the new connections on my SocketIO server.