godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.51k stars 21.26k forks source link

WebsocketClient.poll() sometime can crash a thread. #28602

Closed fian46 closed 5 years ago

fian46 commented 5 years ago

as state in title

akien-mga commented 5 years ago

The sarcasm is not necessary.

Please be more specific, the issue title says "sometime" and the steps to reproduce are "just call poll()".

Is it random? Does it happen only in some situations? What situations? How do you set up your thread and WebSocketClient?

What do you mean by crash? The game crashes with a backtrace?

fian46 commented 5 years ago

wow i dont think it sarcasm. english is not my native language. if you think so i am sorry. i will edit the point where you think it sarcasm.

'crash the thread' the game is not crash the thread is just not running. and hang forever in poll(). i check is_active is always true. means it still running. but it is not.

is it random ? yes that is why i put 'sometimes'. backtrace ? you mean stack trace ? there is nothing in gdscript debugger

fian46 commented 5 years ago

step to reproduce i just loop forever and call pool() in thread

Faless commented 5 years ago

step to reproduce i just loop forever and call pool() in thread

Please note that network functions are not thread safe. If you are accessing it from different threads, you need to properly protect critical sections with a mutex.

@fian46 Can you provide a minimal reproduction project?

fian46 commented 5 years ago

WSRepro.zip i am sure i use mutex here. i never touch socket when write and read message with game thread only network thread can touch socket and passing message with guarded shared object. but still it crash the thread.

fian46 commented 5 years ago

minimal reproduction project need websocket server running and send pong message back after the project send ping to server.

fian46 commented 5 years ago

WSRepro.zip wrong files use this i am sorry

Faless commented 5 years ago

'crash the thread' the game is not crash the thread is just not running. and hang forever in poll(). i check is_active is always true. means it still running. but it is not.

That's still not a crash.

In any case, I added prints here an there to your reproduction project all seems to work fine (I've been running ~10 mins now). Note that in your project the Timer is set as oneshot so it sends the data only once (I changed it not to be oneshot so I get constant 1 sec pings)

fian46 commented 5 years ago

i dont know sometimes it stop poll() forever. no shutdown log print in debugger.

Faless commented 5 years ago

If it stops polling you should get a disconnect in the server after few minutes. Do you see the disconnect in the server?

On Thu, May 2, 2019, 18:50 fian46 notifications@github.com wrote:

i dont know sometimes it stop poll() forever. no shutdown log print in debugger.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/28602#issuecomment-488747650, or mute the thread https://github.com/notifications/unsubscribe-auth/AAM4C3WKMJVJDPQRSDQUMCTPTMLWNANCNFSM4HJ5HXAQ .

fian46 commented 5 years ago

server never get disconnect from client.

Faless commented 5 years ago

Then it seems like poll is working, why you think it's not? Did you try stopping the server and see if the client disconnects?

On Thu, May 2, 2019, 19:15 fian46 notifications@github.com wrote:

server never get disconnect from client.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/28602#issuecomment-488755988, or mute the thread https://github.com/notifications/unsubscribe-auth/AAM4C3RSTG4PQGBT6DY67H3PTMOU3ANCNFSM4HJ5HXAQ .

fian46 commented 5 years ago

but the message is never write to server. the server never get any message. i also put log in client loop poll() to check if loop is still running in thread but it stop print anything. i assume thread already crash

fian46 commented 5 years ago

after it stuck client still connected but thread stop running. server detect client still connected but no message arrive. when i close client the server detect connection closed. it means no poll anymore.

Faless commented 5 years ago

As I said, if you are using the reproduction project you sent me:

The data is only sent one, because the timer is oneshot. The print inside the poll is outside the while loop, so it will only be print once.

On Thu, May 2, 2019, 19:35 fian46 notifications@github.com wrote:

after it stuck client still connected but thread stop running. server detect client still connected but no message arrive. when i close client the server detect connection closed. it means no poll anymore.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/28602#issuecomment-488762338, or mute the thread https://github.com/notifications/unsubscribe-auth/AAM4C3RZLCL77J2SA2JNLGTPTMQ4VANCNFSM4HJ5HXAQ .

fian46 commented 5 years ago

my bad can you make it not one shot ? and test it wit wait time 0.1 ?

Faless commented 5 years ago

Did you try? I tried without lowering the time (left 1 sec) and it was working.

On Thu, May 2, 2019, 19:48 fian46 notifications@github.com wrote:

my bad can you make it not one shot ? and test it wit wait time 0.1 ?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/28602#issuecomment-488767114, or mute the thread https://github.com/notifications/unsubscribe-auth/AAM4C3RQMWTRKDJ45LOG4FDPTMSQNANCNFSM4HJ5HXAQ .

fian46 commented 5 years ago

so it hang ? and thread is killed but socket still connected right ?

Faless commented 5 years ago

No, it doesn't hang, it works as expected

On Thu, May 2, 2019, 20:09 fian46 notifications@github.com wrote:

so it hang ? and thread is killed but socket still connected right ?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/28602#issuecomment-488774238, or mute the thread https://github.com/notifications/unsubscribe-auth/AAM4C3Q545FKJGBX5NQRCF3PTMU5RANCNFSM4HJ5HXAQ .

Faless commented 5 years ago

It also works with wait time 0.1.

ryanhaticus commented 5 years ago

WebSockets in Godot randomly spit out this error https://cdn.discordapp.com/attachments/583800946639896579/584497147458617350/error1.PNG causing our game to crash. Not sure at all, we need an answer ASAP. Our game jam ends today!

Faless commented 5 years ago

@ryanhuellen hard to tell the cause, it seems like you are trying to poll a websocket that's been closed or failed to connect. Are you checking that connect_to_url returns OK? Are you connected to the various signals (connection_closed, connection_error)? Are you receiving any of them? Can you provide a minimum reproduction project?

Faless commented 5 years ago

Closing, not really a crash, not apparently a bug, just a bogus game script.