mmo-ref/tinymux
1
0
Fork 0
mirror of https://github.com/brazilofmux/tinymux synced 2026-08-13 00:23:11 -04:00
Code Issues Projects Releases Packages Wiki Activity

fix(net): contain exceptions on the accept branch of run_main_loop (#2018)

Browse source 
The last unguarded route into the event loop. #2010 wrapped
handleNetworkEvent and #2022 wrapped the protocol-detect finalize; both are
in run_main_loop's established-connection branch. The accept branch is a
different branch of the same loop and had no handler, nor does anything
above it -- run_main_loop <- ganl_main_loop <- driver.cpp, which has no
catch -- so a throw there reached std::terminate.

Everything in that sequence allocates: createConnection is a make_shared,
the four map insertions allocate nodes (one copying a std::string), and
initialize() reaches onConnectionOpen -> allocate_desc -> init_desc.

Demonstrated with a counting bad_alloc injection in init_desc, gated on an
environment variable so the baseline and injected runs are the same binary.
Firing on the *second* accept rather than the first: the harness's own
readiness probe is an accept and would otherwise consume a one-shot, which
both hides the result and leaves "it was broken from startup" unexcluded.

  case                        before                    after
  baseline, no injection      725 bytes, alive          unchanged
  inject within 15s           DEAD, stayed dead         alive, 725 bytes
  inject past 15s             died, re-execed           alive, 725 bytes

The two injected rows differ before the fix because #2012's SIGABRT arm is
gated on bCanRestart, which arms 15s after startup. Inside that window the
game died and stayed dead. Past it the game survived -- by forking, coring
and re-execing, which drops every connected session for one bad accept.
Neither is acceptable when the fault is confined to a single connection.

After the fix, both injected cases keep the server up with only the
offending connection lost, one game log rather than two (no restart), and
the barrier recorded:

  GANL: exception accepting handle 9 (std::bad_alloc); dropping the connection.

accept_cleanup_contained cannot throw, per the rule #2010 set with
close_contained and #2022 had to be corrected on: a barrier whose recovery
can abort is not a barrier. It erases whichever of the four maps got
populated -- erasing an absent key is a no-op, so it is correct wherever
the throw landed without having to know -- and closes the fd, which is
otherwise leaked because onConnectionClose never runs for a connection that
never opened.

Verified on macOS/arm64: make test 35 passed / 1 skipped / 0 failed
(jit=yes stubslave=no nls=yes realitylvls=yes wodrealms=yes).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Stephen Dennis 2026-08-03 23:36:02 -06:00
parent a9f1a839f3
commit 39fa3f4f4e
2 changed files with 105 additions and 28 deletions
Download patch file Download diff file Expand all files Collapse all files

5
mux/include/ganl_adapter.h
View file

@ -53,6 +53,11 @@ public:
void prepare_for_restart();
void run_main_loop();
// Undo a partially-completed accept after a throw (#2018). Cannot
// itself throw: it runs from inside a catch, and a barrier whose
// recovery can abort is not a barrier.
void accept_cleanup_contained(ganl::ConnectionHandle handle);
bool restarting_{false};
// --- TinyMUX Interface ---

128
mux/src/ganl_adapter.cpp
View file

@ -2300,6 +2300,37 @@ static void shutdown_contained(DESC *d, unsigned long long handle)
}
}
// Undo a partially-completed accept (#2018). Runs from inside a catch, so
// it must not throw -- same rule as close_contained above.
//
// Erasing a key that was never inserted is a no-op, so this is correct
// wherever in the accept sequence the throw landed, without having to know.
// The fd close matters: onConnectionClose only runs for a connection that
// opened, so a throw part-way through leaks the descriptor otherwise.
void GanlAdapter::accept_cleanup_contained(ganl::ConnectionHandle handle)
{
try
{
handle_to_conn_.erase(handle);
connection_listener_map_.erase(handle);
pending_remote_addresses_.erase(handle);
pending_tls_flags_.erase(handle);
networkEngine_->closeConnection(handle);
}
catch (const std::exception &e)
{
g_pILog->WriteString(tprintf(
T("GANL: exception cleaning up handle %llu (%s); cleanup abandoned.\n"),
static_cast<unsigned long long>(handle), e.what()));
}
catch (...)
{
g_pILog->WriteString(tprintf(
T("GANL: unknown exception cleaning up handle %llu; cleanup abandoned.\n"),
static_cast<unsigned long long>(handle)));
}
}
void GanlAdapter::run_main_loop() {
g_pILog->WriteString(T("GANL: Entering main loop.\n"));
g_pILog->Flush();
@ -2474,36 +2505,77 @@ void GanlAdapter::run_main_loop() {
useTls = ctx->is_ssl;
}
std::shared_ptr<ganl::ConnectionBase> conn = ganl::ConnectionFactory::createConnection(
connHandle,
*networkEngine_,
secureTransport_.get(),
*protocolHandler_,
*sessionManager_);
// Exception barrier (#2018). Everything from here to the
// end of the accept allocates: createConnection is a
// make_shared, the four map insertions allocate nodes (one
// copying a std::string), and initialize() reaches
// onConnectionOpen -> allocate_desc -> init_desc. This is
// the accept branch of run_main_loop, and unlike the
// established-connection branch below it had no handler --
// nor does anything above it, since run_main_loop <-
// ganl_main_loop <- driver.cpp has no catch. A throw here
// reached std::terminate.
//
// Demonstrated by injecting a one-shot bad_alloc in
// init_desc: inside the first 15s the game died and stayed
// dead, and past that window it took the #2012 SIGABRT arm
// and re-execed -- surviving, but dropping every session
// for one bad accept. Contain it to the one connection.
//
// Cleanup mirrors the initialize()-failed path below: erase
// whichever of the four maps got populated (erase of an
// absent key is a no-op, so this is correct wherever the
// throw landed) and close the fd, which is otherwise leaked
// because onConnectionClose never runs for a connection
// that never opened.
//
try
{
std::shared_ptr<ganl::ConnectionBase> conn = ganl::ConnectionFactory::createConnection(
connHandle,
*networkEngine_,
secureTransport_.get(),
*protocolHandler_,
*sessionManager_);
if (!conn) {
g_pILog->WriteString(tprintf(T("GANL: Failed to allocate ConnectionBase for handle %llu\n"),
static_cast<unsigned long long>(connHandle)));
networkEngine_->closeConnection(connHandle);
continue;
if (!conn) {
g_pILog->WriteString(tprintf(T("GANL: Failed to allocate ConnectionBase for handle %llu\n"),
static_cast<unsigned long long>(connHandle)));
networkEngine_->closeConnection(connHandle);
continue;
}
connection_listener_map_[connHandle] = listenerCtx;
pending_remote_addresses_[connHandle] = events[i].remoteAddress;
pending_tls_flags_[connHandle] = useTls;
handle_to_conn_[connHandle] = conn;
// Count only connections that fully initialize. Failed
// init closes the fd itself and never reaches
// onConnectionClose, so accepting first would permanently
// inflate NET/STAT live = accepted - closed.
if (!conn->initialize(useTls)) {
handle_to_conn_.erase(connHandle);
connection_listener_map_.erase(connHandle);
pending_remote_addresses_.erase(connHandle);
pending_tls_flags_.erase(connHandle);
} else {
connections_accepted_++;
}
}
connection_listener_map_[connHandle] = listenerCtx;
pending_remote_addresses_[connHandle] = events[i].remoteAddress;
pending_tls_flags_[connHandle] = useTls;
handle_to_conn_[connHandle] = conn;
// Count only connections that fully initialize. Failed
// init closes the fd itself and never reaches
// onConnectionClose, so accepting first would permanently
// inflate NET/STAT live = accepted - closed.
if (!conn->initialize(useTls)) {
handle_to_conn_.erase(connHandle);
connection_listener_map_.erase(connHandle);
pending_remote_addresses_.erase(connHandle);
pending_tls_flags_.erase(connHandle);
} else {
connections_accepted_++;
catch (const std::exception &e)
{
g_pILog->WriteString(tprintf(
T("GANL: exception accepting handle %llu (%s); dropping the connection.\n"),
static_cast<unsigned long long>(connHandle), e.what()));
accept_cleanup_contained(connHandle);
}
catch (...)
{
g_pILog->WriteString(tprintf(
T("GANL: unknown exception accepting handle %llu; dropping the connection.\n"),
static_cast<unsigned long long>(connHandle)));
accept_cleanup_contained(connHandle);
}
}
continue;