scp-fs2open / scp-fs2open/fs2open.github.com

Standalone server crashes when client disconnects before mission loads (SEXP nodes uninitialized)

Open
#7,353 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
C++
Stars
487
Forks
184
PR merge metrics
PR metrics pending

Description

## Summary

A standalone server crashes with `Assert: "node != -1"` in `get_sexp()` at `sexp.cpp:4803` when a client connects to the server and then backs out before a mission is ever loaded. This is 100% reproducible, but depends on certain scripting hooks in use (see below).

## Steps to Reproduce

1. Start a standalone server (with any mod that registers an `On Mission End` or `On Mission About To End` hook calling `mn.evaluateSEXP()`, or use the attached test script)
2. From a client, go to Multiplayer and browse the Games list
3. Select the standalone server
4. On the "name your session" screen, press Escape to back out before proceeding
5. The standalone server immediately crashes

A minimal reproduction script (`test_sexp_crash-sct.tbm`) that triggers this with retail FS2 data:

```lua
#Conditional Hooks
$Application: FS2_Open

$On Mission End:
[
mn.evaluateSEXP("( true )")
]

#End
```

Drop this into data/tables/ on the standalone's installation.

**Root Cause**

When a client disconnects early, the standalone follows this call chain:

```
multi_endgame_process()
→ multi_endgame_cleanup() (multi_endgame.cpp:370)
→ multi_standalone_reset_all() (multi.cpp:1670)
→ game_level_close() (freespace.cpp:1004)
→ OnMissionAboutToEndHook (freespace.cpp:912)
→ OnMissionEndHook (freespace.cpp:999)
```

game_level_close() fires the OnMissionAboutToEnd and OnMissionEnd scripting hooks unconditionally. If a mod's Lua script calls mn.evaluateSEXP() from one of these hooks, it enters run_sexp() → get_sexp_main() → get_sexp() → alloc_sexp().

The problem is that no mission was ever loaded, so init_sexp() was never called. This means:

- Locked_sexp_true and Locked_sexp_false are still -1 (their initial values from declaration at sexp.cpp:1018-1019)
- When alloc_sexp() encounters a true or false token, it short-circuits and returns Locked_sexp_true / Locked_sexp_false directly (sexp.cpp:1435-1439), which is -1
- This propagates back to get_sexp() where Assert(node != -1) fires at sexp.cpp:4802

Note: client-hosted multiplayer does not hit this because multi_endgame_cleanup() only calls multi_standalone_reset_all() → game_level_close() for standalone servers (multi_endgame.cpp:368-370). The client-hosted path simply navigates back to the lobby UI without any level teardown.

**Analysis**

The multi_standalone_reset_all() → game_level_close() path is original FSO code (~23 years old) and was always safe because game_level_close() only tore down game subsystems that tolerated being called on uninitialized state. The addition of scripting hooks (OnMissionAboutToEnd / OnMissionEnd) to game_level_close() introduced a new assumption — that
the SEXP system and other subsystems are initialized when those hooks fire — that doesn't hold when no mission was ever loaded.

**Suggested Fix**

Two complementary guards:

1. multi_standalone_reset_all(): Skip the call to game_level_close() if no mission was ever loaded (e.g., check a game state flag or whether init_sexp() has been called)
2. game_level_close(): Skip firing the scripting hooks if the SEXP system isn't initialized (belt-and-suspenders defense so any future caller of game_level_close() in an unexpected state doesn't crash)

**Stack Trace**

```
#0 __pthread_kill_implementation () at /lib64/libc.so.6
#1 raise () at /lib64/libc.so.6
#2 abort () at /lib64/libc.so.6
#3 os::dialogs::Error (text="Assert: \"node != -1\" ...") at osapi/dialogs.cpp:291
#4 os::dialogs::AssertMessage (text="node != -1", filename="sexp.cpp", linenum=4803) at osapi/dialogs.cpp:138
#5 get_sexp () at parse/sexp.cpp:4803
#6 get_sexp () at parse/sexp.cpp:4587
#7 get_sexp_main () at parse/sexp.cpp:31012
#8 run_sexp (sexpression="( hud-set-custom-gauge-active ( false ) !CustomRoleGauge_Frame! )") at parse/sexp.cpp:31054
#9 scripting::api::l_Mission_evaluateSEXP_f () at scripting/api/libs/mission.cpp:204
#10-#16 [lua calls]
#17 script_state::RunBytecode () at scripting/scripting.cpp:789
#18 script_state::RunCondition (action_type=76) at scripting/scripting.cpp:508
#19 scripting::HookImpl::run () at scripting/hook_api.h:204
#20 game_level_close () at freespace2/freespace.cpp:915
#21 multi_standalone_reset_all () at network/multi.cpp:1670
#22 multi_endgame_cleanup () at network/multi_endgame.cpp:370
#23 multi_endgame_process () at network/multi_endgame.cpp:135
#24 multi_do_frame () at network/multi.cpp:1173
#25-#30 [game loop]
```

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.