godotengine / godot

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

Enet Multiplayer crashes when server disconnects #27316

Closed Dragoncraft89 closed 4 years ago

Dragoncraft89 commented 5 years ago

Godot version:

Godot 3.1

OS/device including version:

Debian GNU/Linux 9

Issue description:

Client sometimes segfaults, when server application is closed while a client is still connected. Backtrace:

handle_crash: Program crashed with signal 11
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[1] /lib/x86_64-linux-gnu/libc.so.6(+0x37940) [0x7fab69f3a940] (??:0)
[2] godot() [0xba98b2] (??:?)
[3] godot() [0x925169] (<artificial>:?)
[4] godot() [0x96cf80] (<artificial>:?)
[5] godot() [0x23e3c45] (??:?)
[6] godot() [0x16cb9ec] (??:?)
[7] godot() [0xaf1220] (??:?)
[8] godot() [0x7b28ed] (??:?)
[9] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xeb) [0x7fab69f2709b] (??:0)
[10] godot() [0x7bfc4e] (??:?)
-- END OF BACKTRACE --

It seems to segfault at NetworkedMultiplayerEnet::_pop_current_package, because current_package.packet is an invalid pointer.

It happens very often if in the server_connected signal handler get_tree().network_peer is set to null (because it triggers the destructor)

Steps to reproduce: Connect a client to a server and close the server connection. Does not always happen but quite frequently.

Minimal reproduction project:

I'm not able to produce a minimal project, therefore I'll attach my current project (fortunately it's not that big yet): Starfall.zip.

The file of interest is lobby.gd

realkotob commented 5 years ago

Not sure if I'm having the same thing, but occasionally my server crashes when a client disconnects .

ERROR: _get_socket_error: Socket error: 10054
   At: drivers/unix/net_socket_posix.cpp:202
ERROR: put_packet: Condition ' !is_connected_to_host() ' is true. returned: FAILED
   At: modules/websocket/wsl_peer.cpp:238
Client 1252280632 was unregistered
ERROR: put_packet: Condition ' !is_connected_to_host() ' is true. returned: FAILED
   At: modules/websocket/wsl_peer.cpp:238

It's a bit depressing because I really don't want my server crashing, if anyone can suggest any workaround it'd be really helpful.

Faless commented 5 years ago

@asheraryam do you have the stack trace? What version of godot are you running?

EDIT: you also seem to be using WebSocket, not ENet, so maybe it's better to open a new issue.

Faless commented 4 years ago

Related to #33788 .

In this case, the user code uses a yield on a local reference. When the yielding function finally ends, it frees the reference during the signal it's been waiting for.

An extract


class Connection:
    signal await_connection(err)

    func _init(tree):
        tree.connect("connection_failed", self, "_failure")
        tree.connect("connected_to_server", self, "_success")

    func _failure():
        emit_signal("await_connection", ERR_TIMEOUT)

    func _success():
        emit_signal("await_connection", OK)

func _on_Connect_pressed():
    var host = NetworkedMultiplayerENet.new()
    var conn = Connection.new(get_tree())
    var err = host.create_client($IP/LineEdit.text, PORT)
    if err != OK:
        host.close_connection()
        return

    get_tree().network_peer = host
    err = yield(conn, "await_connection")
    if err != OK:
        host.close_connection()
        return
    # By returning, `conn` is freed during the `await_connection` signal.
akien-mga commented 4 years ago

Is this still reproducible in the latest 3.2 builds?

Faless commented 4 years ago

Possibly related to #33290. needs retest

Dragoncraft89 commented 4 years ago

@akien-mga Sorry for the delay,

I was unable to reproduce it in my project with 3.2, so I assume this is fixed

Dragoncraft89 commented 4 years ago

I just realized that I now get an error when trying to set the network_peer to null, when the connection is dropped, as there is currently a signal being emitted by that object.

This possibly makes it harder to reproduce, as triggering the destructor increased the chance of a segfault by a lot (in 3.1). I assume godot now prohibits the destructor from running while the signal is still being emitted? (This possibly could have been the observed bug too)

Maybe this bug is not fixed after all, but instead is just hard to reproduce as in 3.1 this has been happening without setting the network_peer to null in the disconnect signal, but very rarely. However I do not have any evidence of this, so there's no point in keeping this open.

majikayogames commented 6 months ago

enet crash minimal.zip

I think this is the same bug, client crashes if multiplayer peer set to null. It occurs on 4.2.2 stable if ENet client has multiplayer_peer set to null or OfflineMultiplayerPeer.new() on peer disconnected signal.

At first I thought I could crash clients by setting it on server but I think this is just if the client sets their multiplayer_peer to null on disconnect.