From c66f8e1d8eba00343583802a1890fd544db9fab0 Mon Sep 17 00:00:00 2001 From: Nicolas 'Pixel' Noble Date: Tue, 28 Jul 2026 15:42:06 -0700 Subject: [PATCH 01/13] UvFifoListener: report bind and listen failures. The bind and listen both run on the uv worker thread, so a failure could not be returned from start(); all three error paths closed the server handle with an empty callback and told nobody. That left the listener in a state where a subsequent stop() closed the same handle a second time and tripped uv_close's !uv__is_closing assertion, which is reachable from the GUI by enabling a server on a busy port and switching it back off. Give the listener a status and the uv error code, and route the failure through the same nullptr callback an orderly shutdown uses, so consumers do not need a second teardown path and stop leaking their async handle when the bind fails. Signed-off-by: Nicolas 'Pixel' Noble --- src/support/uvfile.cc | 29 ++++++- src/support/uvfile.h | 21 +++++ tests/support/uvfifolistener.cc | 131 ++++++++++++++++++++++++++++++++ 3 files changed, 178 insertions(+), 3 deletions(-) create mode 100644 tests/support/uvfifolistener.cc diff --git a/src/support/uvfile.cc b/src/support/uvfile.cc index 2a14a6e97..5a84e45ef 100644 --- a/src/support/uvfile.cc +++ b/src/support/uvfile.cc @@ -925,11 +925,25 @@ void PCSX::UvFifo::write(Slice &&slice) { }); } +void PCSX::UvFifoListener::failed(int code) { + m_lastErrorCode.store(code, std::memory_order_release); + uv_close(reinterpret_cast(&m_server), [](uv_handle_t *handle) { + UvFifoListener *listener = reinterpret_cast(handle->data); + listener->m_status.store(Status::Failed, std::memory_order_release); + // Same wake-up an orderly stop() produces, so the consumer's nullptr + // branch runs and closes its async instead of leaking it. + listener->m_pending.Enqueue(nullptr); + uv_async_send(listener->m_async); + }); +} + void PCSX::UvFifoListener::start(unsigned port, uv_loop_t *loop, uv_async_t *async, std::function &&cb) { m_cb = std::move(cb); async->data = this; m_async = async; + m_lastErrorCode.store(0, std::memory_order_release); + m_status.store(Status::Starting, std::memory_order_release); uv_async_init(loop, async, [](uv_async_t *async) { UvFifoListener *self = reinterpret_cast(async->data); UvFifo *fifo = nullptr; @@ -944,12 +958,12 @@ void PCSX::UvFifoListener::start(unsigned port, uv_loop_t *loop, uv_async_t *asy struct sockaddr_in bindAddr; int result = uv_ip4_addr("0.0.0.0", port, &bindAddr); if (result != 0) { - uv_close(reinterpret_cast(&m_server), [](uv_handle_t *handle) {}); + failed(result); return; } result = uv_tcp_bind(&m_server, reinterpret_cast(&bindAddr), 0); if (result != 0) { - uv_close(reinterpret_cast(&m_server), [](uv_handle_t *handle) {}); + failed(result); return; } result = uv_listen((uv_stream_t *)&m_server, 16, [](uv_stream_t *server, int status) { @@ -968,16 +982,25 @@ void PCSX::UvFifoListener::start(unsigned port, uv_loop_t *loop, uv_async_t *asy } }); if (result != 0) { - uv_close(reinterpret_cast(&m_server), [](uv_handle_t *handle) {}); + failed(result); return; } + m_status.store(Status::Listening, std::memory_order_release); }); } void PCSX::UvFifoListener::stop() { request([this](auto loop) { + // Runs on the same worker thread as start()'s body and after it, so the + // status here is settled: a failed bind has already closed m_server and + // there is nothing left to tear down. Closing it again is what aborted + // inside uv_close (`!uv__is_closing(handle)`) whenever a server was + // enabled on a busy port and then switched off. + auto status = m_status.load(std::memory_order_acquire); + if ((status == Status::Failed) || (status == Status::Stopped)) return; uv_close(reinterpret_cast(&m_server), [](uv_handle_t *handle) { UvFifoListener *listener = reinterpret_cast(handle->data); + listener->m_status.store(Status::Stopped, std::memory_order_release); listener->m_pending.Enqueue(nullptr); uv_async_send(listener->m_async); }); diff --git a/src/support/uvfile.h b/src/support/uvfile.h index a2b8e871a..6641ee7cd 100644 --- a/src/support/uvfile.h +++ b/src/support/uvfile.h @@ -259,16 +259,37 @@ class UvFifo : public File, public UvThreadOp { class UvFifoListener : public UvThreadOp { public: + // The bind and the listen both happen asynchronously on the uv worker + // thread, so a failure cannot be returned from start(). It lands here + // instead, and the callback is invoked with nullptr the same way an + // orderly shutdown does, so consumers have exactly one path for "this + // listener is finished, clean up". + enum class Status { Stopped, Starting, Listening, Failed }; + UvFifoListener() {} void start(unsigned port, uv_loop_t* loop, uv_async_t* async, std::function&& cb); void stop(); + Status status() const { return m_status.load(std::memory_order_acquire); } + bool isListening() const { return status() == Status::Listening; } + // uv error code of whichever step failed, or 0. Meaningful once status() is Failed. + int lastErrorCode() const { return m_lastErrorCode.load(std::memory_order_acquire); } + const char* lastError() const { + int code = lastErrorCode(); + return code == 0 ? "" : uv_strerror(code); + } + private: virtual bool canCache() const override { return false; } + // Worker-thread only. Records the error, tears down the half-built server + // handle, and wakes the consumer with a nullptr. + void failed(int code); uv_async_t* m_async = nullptr; uv_tcp_t m_server = {}; std::function m_cb; ConcurrentQueue m_pending; + std::atomic m_status = Status::Stopped; + std::atomic m_lastErrorCode = 0; }; } // namespace PCSX diff --git a/tests/support/uvfifolistener.cc b/tests/support/uvfifolistener.cc new file mode 100644 index 000000000..b2e3e3fbe --- /dev/null +++ b/tests/support/uvfifolistener.cc @@ -0,0 +1,131 @@ +/*************************************************************************** + * Copyright (C) 2026 PCSX-Redux authors * + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + * This program is distributed in the hope that it will be useful, * + * but WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * + * GNU General Public License for more details. * + * * + * You should have received a copy of the GNU General Public License * + * along with this program; if not, write to the * + * Free Software Foundation, Inc., * + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. * + ***************************************************************************/ + +#include + +#include +#include + +#include "gtest/gtest.h" +#include "support/uvfile.h" + +using namespace PCSX; + +namespace { + +// Ports picked high and odd to reduce the odds of colliding with anything real +// on a developer box or CI runner. +constexpr unsigned c_squattedPort = 47821; +constexpr unsigned c_freePort = 47823; + +// Pump the caller-side loop for a while, giving the uv worker thread time to +// service the queued request() and hand anything back through the async. +void pump(uv_loop_t* loop, int milliseconds = 250) { + auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(milliseconds); + while (std::chrono::steady_clock::now() < deadline) { + uv_run(loop, UV_RUN_NOWAIT); + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + } +} + +// Takes a port and holds it, so UvFifoListener's bind is guaranteed to fail. +struct Squatter { + explicit Squatter(uv_loop_t* loop, unsigned port) { + uv_tcp_init(loop, &m_tcp); + struct sockaddr_in addr; + EXPECT_EQ(uv_ip4_addr("0.0.0.0", port, &addr), 0); + EXPECT_EQ(uv_tcp_bind(&m_tcp, reinterpret_cast(&addr), 0), 0); + EXPECT_EQ(uv_listen(reinterpret_cast(&m_tcp), 16, [](uv_stream_t*, int) {}), 0); + } + ~Squatter() { uv_close(reinterpret_cast(&m_tcp), [](uv_handle_t*) {}); } + uv_tcp_t m_tcp = {}; +}; + +} // namespace + +// Control: a listener that binds cleanly must start and stop without incident. +// If this one ever fails, the busy-port test below proves nothing. +TEST(UvFifoListener, StartStopOnFreePort) { + UvThreadOp::UvThread uvThread; + uv_loop_t loop; + uv_loop_init(&loop); + + int nullptrCallbacks = 0; + UvFifoListener listener; + uv_async_t async = {}; + listener.start(c_freePort, &loop, &async, [&nullptrCallbacks](UvFifo* fifo) { + if (!fifo) nullptrCallbacks++; + }); + pump(&loop); + + EXPECT_EQ(listener.status(), UvFifoListener::Status::Listening); + EXPECT_TRUE(listener.isListening()); + EXPECT_EQ(listener.lastErrorCode(), 0); + EXPECT_EQ(nullptrCallbacks, 0); + + listener.stop(); + pump(&loop); + + EXPECT_EQ(listener.status(), UvFifoListener::Status::Stopped); + EXPECT_EQ(nullptrCallbacks, 1); + + uv_run(&loop, UV_RUN_NOWAIT); + uv_loop_close(&loop); +} + +// The bug: start() on an occupied port hits the uv_tcp_bind failure path, which +// uv_close()es m_server and tells nobody. Nothing in UvFifoListener records that +// it is already closing, so a subsequent stop() - which is exactly what the GUI +// checkbox and the Quitting event both do, since SIO1Server/ATConsServer set +// SERVER_STARTED unconditionally - closes the same handle a second time. +TEST(UvFifoListener, StopAfterFailedBindIsSafe) { + UvThreadOp::UvThread uvThread; + uv_loop_t loop; + uv_loop_init(&loop); + + Squatter squatter(&loop, c_squattedPort); + + int nullptrCallbacks = 0; + UvFifoListener listener; + uv_async_t async = {}; + listener.start(c_squattedPort, &loop, &async, [&nullptrCallbacks](UvFifo* fifo) { + if (!fifo) nullptrCallbacks++; + }); + pump(&loop); + + // The failure has to be visible, and it has to be the *right* failure - + // an unreported bind error and a bind error reported as the wrong code are + // both things a status bullet would render identically. + EXPECT_EQ(listener.status(), UvFifoListener::Status::Failed); + EXPECT_FALSE(listener.isListening()); + EXPECT_EQ(listener.lastErrorCode(), UV_EADDRINUSE); + EXPECT_STREQ(listener.lastError(), uv_strerror(UV_EADDRINUSE)); + // Consumers learn about it through the same nullptr they get on shutdown, + // so nobody has to grow a second teardown path. + EXPECT_EQ(nullptrCallbacks, 1); + + // This is the line that used to abort inside uv_close. + listener.stop(); + pump(&loop); + + EXPECT_EQ(listener.status(), UvFifoListener::Status::Failed); + + uv_run(&loop, UV_RUN_NOWAIT); + uv_loop_close(&loop); +} From 55e8a50ca5f6db98690659371451667ca3a923e9 Mon Sep 17 00:00:00 2001 From: Nicolas 'Pixel' Noble Date: Tue, 28 Jul 2026 15:49:48 -0700 Subject: [PATCH 02/13] UvFifo: optional readable notification. Reads land in a lock-free queue on the uv worker thread and nothing tells the consumer, so the only way to notice traffic is to poll size() - which is what the SIO1 and ATCons bridges do off the counters. That is fine for them and not fine for anything latency sensitive, so a consumer can now hand over an async living on its own loop and be woken instead. The callback runs on that loop, and fires on arrival as well as on EOF. Installing a notifier on an already readable or already closed fifo fires it once immediately: an accepted connection starts reading the moment it exists on the worker thread, which is well before the consumer receives it through the listener's async, so otherwise the first packet of a connection could sit in the queue until a second one showed up. Signed-off-by: Nicolas 'Pixel' Noble --- src/support/uvfile.cc | 21 +++++++++++ src/support/uvfile.h | 27 ++++++++++++++ tests/support/uvfifolistener.cc | 64 +++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+) diff --git a/src/support/uvfile.cc b/src/support/uvfile.cc index 5a84e45ef..2cf7ef2fa 100644 --- a/src/support/uvfile.cc +++ b/src/support/uvfile.cc @@ -833,8 +833,12 @@ void PCSX::UvFifo::startRead(uv_tcp_t *tcp) { UvFifo *fifo = reinterpret_cast(client->data); if (nread <= 0) { free(fifo->m_buffer); + fifo->m_buffer = nullptr; if (nread < 0) { fifo->m_closed = true; + // A peer hanging up is a state change too; a consumer + // waiting to be woken would otherwise wait forever. + fifo->notify(); } return; } @@ -845,9 +849,26 @@ void PCSX::UvFifo::startRead(uv_tcp_t *tcp) { slice.acquire(b, nread); fifo->m_queue.Enqueue(std::move(slice)); fifo->m_size.fetch_add(nread); + fifo->notify(); }); } +void PCSX::UvFifo::setNotifier(uv_loop_t *loop, uv_async_t *async, std::function &&cb) { + m_notifyCb = std::move(cb); + async->data = this; + uv_async_init(loop, async, [](uv_async_t *async) { + UvFifo *fifo = reinterpret_cast(async->data); + if (fifo->m_notifyCb) fifo->m_notifyCb(); + }); + m_notifyAsync.store(async, std::memory_order_release); + // The fifo may have been readable since before we were installed - an + // accepted connection starts reading on the worker thread the instant it + // exists, well before the consumer gets it back through the listener's + // async. Fire once so that traffic isn't stranded waiting for the next + // packet to arrive. + if ((m_size.load() > 0) || m_closed.load()) notify(); +} + void PCSX::UvFifo::closeInternal() { m_closed.store(true); request([tcp = m_tcp](uv_loop_t *loop) { diff --git a/src/support/uvfile.h b/src/support/uvfile.h index 6641ee7cd..f92c9e007 100644 --- a/src/support/uvfile.h +++ b/src/support/uvfile.h @@ -239,10 +239,35 @@ class UvFifo : public File, public UvThreadOp { virtual bool failed() final override { return m_failed.test(); } bool isConnecting() { return m_connecting.test(); } + // Opt-in readable notification. + // + // Reads land in a lock-free queue on the uv worker thread, so without this + // the only way to notice traffic is to poll size() from somewhere - which + // is what SIO1 and ATCons do off the counters, and which is fine for them. + // It is not fine for anything latency-sensitive or idle-heavy, so a + // consumer can hand over an async living on ITS OWN loop and get woken + // instead. `cb` runs on that loop, never on the worker thread. + // + // Fires on data arrival and on EOF/error. Wake-ups are coalesced by libuv, + // so treat it as "something changed, go look", not as one call per chunk. + // Safe to install after data has already arrived: it fires once up front if + // the fifo is already readable or already closed, so no wake-up is lost in + // the gap between accept and setNotifier. + // + // The caller owns `async` and must keep it alive until it closes the fifo, + // then uv_close it, exactly like UvFifoListener's. + void setNotifier(uv_loop_t* loop, uv_async_t* async, std::function&& cb); + void clearNotifier() { m_notifyAsync.store(nullptr, std::memory_order_release); } + private: virtual void closeInternal() final override; UvFifo(uv_tcp_t*); void startRead(uv_tcp_t*); + // Worker-thread side of the notification. A no-op when nobody opted in. + void notify() { + auto async = m_notifyAsync.load(std::memory_order_acquire); + if (async) uv_async_send(async); + } virtual bool canCache() const override { return false; } uv_tcp_t* m_tcp = nullptr; void* m_buffer = nullptr; @@ -254,6 +279,8 @@ class UvFifo : public File, public UvThreadOp { std::atomic_flag m_connecting; Slice m_slice; size_t m_currentPtr = 0; + std::atomic m_notifyAsync = nullptr; + std::function m_notifyCb; friend class UvFifoListener; }; diff --git a/tests/support/uvfifolistener.cc b/tests/support/uvfifolistener.cc index b2e3e3fbe..ac5bb6af9 100644 --- a/tests/support/uvfifolistener.cc +++ b/tests/support/uvfifolistener.cc @@ -33,6 +33,7 @@ namespace { // on a developer box or CI runner. constexpr unsigned c_squattedPort = 47821; constexpr unsigned c_freePort = 47823; +constexpr unsigned c_notifyPort = 47825; // Pump the caller-side loop for a while, giving the uv worker thread time to // service the queued request() and hand anything back through the async. @@ -129,3 +130,66 @@ TEST(UvFifoListener, StopAfterFailedBindIsSafe) { uv_run(&loop, UV_RUN_NOWAIT); uv_loop_close(&loop); } + +// UvFifo has no readable callback of its own - data lands in a lock-free queue +// on the worker thread and the consumer is expected to poll size(). That is +// what SIO1 and ATCons do off the counters, and it is why porting the GDB and +// web servers onto this transport needs a wake-up first: frame-granularity +// polling would put visible latency on every GDB packet. +// +// This test only passes if uv_async_send actually reached the caller's loop. +// Nothing else runs the callback, so a notifier that never fires fails it. +TEST(UvFifo, ReadableNotifierFires) { + UvThreadOp::UvThread uvThread; + uv_loop_t loop; + uv_loop_init(&loop); + + UvFifo* accepted = nullptr; + UvFifoListener listener; + uv_async_t listenerAsync = {}; + listener.start(c_notifyPort, &loop, &listenerAsync, [&accepted](UvFifo* fifo) { + if (fifo) accepted = fifo; + }); + pump(&loop); + ASSERT_EQ(listener.status(), UvFifoListener::Status::Listening); + + IO client(new UvFifo("127.0.0.1", c_notifyPort)); + pump(&loop); + ASSERT_NE(accepted, nullptr); + IO serverSide(accepted); + + int notifications = 0; + uv_async_t notifyAsync = {}; + accepted->setNotifier(&loop, ¬ifyAsync, [¬ifications]() { notifications++; }); + pump(&loop); + const int baseline = notifications; + + static const char c_payload[] = "hello"; + constexpr size_t c_payloadSize = sizeof(c_payload) - 1; + client->write(c_payload, c_payloadSize); + pump(&loop, 500); + + EXPECT_GT(notifications, baseline); + EXPECT_EQ(serverSide->size(), c_payloadSize); + + char buffer[16] = {}; + EXPECT_EQ(serverSide->read(buffer, c_payloadSize), static_cast(c_payloadSize)); + EXPECT_STREQ(buffer, c_payload); + + // A peer hanging up has to wake the consumer too, or anything waiting on + // the notifier instead of polling never learns the connection is gone. + const int beforeClose = notifications; + client.reset(); + pump(&loop, 500); + EXPECT_GT(notifications, beforeClose); + EXPECT_TRUE(serverSide->isClosed()); + + accepted->clearNotifier(); + serverSide.reset(); + listener.stop(); + pump(&loop); + uv_close(reinterpret_cast(¬ifyAsync), [](uv_handle_t*) {}); + uv_close(reinterpret_cast(&listenerAsync), [](uv_handle_t*) {}); + pump(&loop); + uv_loop_close(&loop); +} From 9b60dfce588ce6f22260e6bef99f41e9bd60ebdc Mon Sep 17 00:00:00 2001 From: Nicolas 'Pixel' Noble Date: Tue, 28 Jul 2026 15:56:16 -0700 Subject: [PATCH 03/13] UvFifo: make a failed connect actionable. m_connecting was set in the constructor and cleared only on the success path, so a fifo that failed to connect reported isConnecting() forever. The SIO1 reconnect button is gated on !connecting() && fifoError(), which means it could never become clickable after the one failure it exists to recover from. Clear the flag on all three failure paths, and record the uv error code while we are here: a failed outgoing connection used to be a bare bool, which is enough to colour a status indicator red and not enough to say why. Wake any installed notifier too, so a consumer waiting on the fifo learns about the failure instead of waiting for a connection that is never coming. Signed-off-by: Nicolas 'Pixel' Noble --- src/support/uvfile.cc | 9 +++++++++ src/support/uvfile.h | 9 +++++++++ tests/support/uvfifolistener.cc | 26 ++++++++++++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/src/support/uvfile.cc b/src/support/uvfile.cc index 2cf7ef2fa..543e15012 100644 --- a/src/support/uvfile.cc +++ b/src/support/uvfile.cc @@ -793,7 +793,10 @@ PCSX::UvFifo::UvFifo(const std::string_view address, unsigned port) : File(File: struct sockaddr_in connectAddr; int result = uv_ip4_addr(host.c_str(), port, &connectAddr); if (result != 0) { + m_connectErrorCode.store(result, std::memory_order_release); + m_connecting.clear(); m_failed.test_and_set(); + notify(); return; } uv_connect_t *connect = new uv_connect_t(); @@ -802,16 +805,22 @@ PCSX::UvFifo::UvFifo(const std::string_view address, unsigned port) : File(File: [](uv_connect_t *connect, int status) { UvFifo *fifo = reinterpret_cast(connect->data); if (status < 0) { + fifo->m_connectErrorCode.store(status, std::memory_order_release); + fifo->m_connecting.clear(); fifo->m_failed.test_and_set(); delete connect; + fifo->notify(); return; } fifo->m_connecting.clear(); fifo->startRead(reinterpret_cast(connect->handle)); }); if (result != 0) { + m_connectErrorCode.store(result, std::memory_order_release); + m_connecting.clear(); m_failed.test_and_set(); delete connect; + notify(); return; } }); diff --git a/src/support/uvfile.h b/src/support/uvfile.h index f92c9e007..fed9c403f 100644 --- a/src/support/uvfile.h +++ b/src/support/uvfile.h @@ -238,6 +238,14 @@ class UvFifo : public File, public UvThreadOp { virtual bool eof() final override { return m_closed.load() && (m_size.load() == 0); } virtual bool failed() final override { return m_failed.test(); } bool isConnecting() { return m_connecting.test(); } + // uv error code of a failed connect, or 0. Meaningful once failed() is set. + // Without this a failed outgoing connection is a bare bool, which is enough + // to colour a status bullet red and not enough to say why. + int connectErrorCode() const { return m_connectErrorCode.load(std::memory_order_acquire); } + const char* connectError() const { + int code = connectErrorCode(); + return code == 0 ? "" : uv_strerror(code); + } // Opt-in readable notification. // @@ -277,6 +285,7 @@ class UvFifo : public File, public UvThreadOp { std::atomic m_size = 0; std::atomic_flag m_failed; std::atomic_flag m_connecting; + std::atomic m_connectErrorCode = 0; Slice m_slice; size_t m_currentPtr = 0; std::atomic m_notifyAsync = nullptr; diff --git a/tests/support/uvfifolistener.cc b/tests/support/uvfifolistener.cc index ac5bb6af9..586c7d21a 100644 --- a/tests/support/uvfifolistener.cc +++ b/tests/support/uvfifolistener.cc @@ -34,6 +34,7 @@ namespace { constexpr unsigned c_squattedPort = 47821; constexpr unsigned c_freePort = 47823; constexpr unsigned c_notifyPort = 47825; +constexpr unsigned c_deadPort = 47827; // Pump the caller-side loop for a while, giving the uv worker thread time to // service the queued request() and hand anything back through the async. @@ -193,3 +194,28 @@ TEST(UvFifo, ReadableNotifierFires) { pump(&loop); uv_loop_close(&loop); } + +// A failed outgoing connection has to end up in a state the UI can act on. +// Both halves of this matter and only one of them is about the error string: +// m_connecting was set in the constructor and cleared ONLY on the success +// path, so a fifo that failed to connect reported isConnecting() forever. The +// SIO1 "Reconnect" button - the single piece of network error UI in the whole +// emulator - is gated on `!connecting() && fifoError()`, so it could never +// become clickable after exactly the failure it exists to recover from. +TEST(UvFifo, FailedConnectIsActionable) { + UvThreadOp::UvThread uvThread; + + // Nothing is listening here; c_deadPort is never bound by any test. + IO client(new UvFifo("127.0.0.1", c_deadPort)); + + auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + auto fifo = client.asA(); + while (fifo->isConnecting() && (std::chrono::steady_clock::now() < deadline)) { + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + } + + EXPECT_TRUE(fifo->failed()); + EXPECT_FALSE(fifo->isConnecting()); + EXPECT_EQ(fifo->connectErrorCode(), UV_ECONNREFUSED); + EXPECT_STREQ(fifo->connectError(), uv_strerror(UV_ECONNREFUSED)); +} From 01106765685d3187852fddf229cab9fbcc784e84 Mon Sep 17 00:00:00 2001 From: Nicolas 'Pixel' Noble Date: Tue, 28 Jul 2026 15:59:51 -0700 Subject: [PATCH 04/13] Add a common base for network endpoints. Every server in the emulator hand-rolls the same uv_tcp_init / uv_ip4_addr / uv_tcp_bind / uv_listen sequence, declares its own three-state status enum with its own spelling, and has no way to express failure. Nothing reads any of those enums. Introduce Network::Endpoint with a single Status vocabulary and a last error, plus Network::Server on top of UvFifoListener and Network::Client on top of UvFifo. A concrete endpoint now says what to do with a connection and nothing else; connections arrive as IO, so there is no per-server accept, read, or write-queue machinery to retype. Endpoints self-register, so the configuration UI can iterate them rather than hard-coding a row per service, and a new one gets a status indicator without touching the window. Signed-off-by: Nicolas 'Pixel' Noble --- src/support/network.cc | 158 ++++++++++++++++++++++++++++ src/support/network.h | 160 ++++++++++++++++++++++++++++ tests/support/network.cc | 220 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 538 insertions(+) create mode 100644 src/support/network.cc create mode 100644 src/support/network.h create mode 100644 tests/support/network.cc diff --git a/src/support/network.cc b/src/support/network.cc new file mode 100644 index 000000000..faf6ca63a --- /dev/null +++ b/src/support/network.cc @@ -0,0 +1,158 @@ +/*************************************************************************** + * Copyright (C) 2026 PCSX-Redux authors * + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + * This program is distributed in the hope that it will be useful, * + * but WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * + * GNU General Public License for more details. * + * * + * You should have received a copy of the GNU General Public License * + * along with this program; if not, write to the * + * Free Software Foundation, Inc., * + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. * + ***************************************************************************/ + +#include "support/network.h" + +#include + +namespace PCSX::Network { + +const char* toString(Status status) { + switch (status) { + case Status::Stopped: + return "Stopped"; + case Status::Starting: + return "Starting"; + case Status::Running: + return "Running"; + case Status::Failed: + return "Failed"; + } + return "Unknown"; +} + +// Function-local so registration during static construction is safe regardless +// of translation unit ordering. +static std::vector& registry() { + static std::vector s_registry; + return s_registry; +} + +const std::vector& Endpoint::all() { return registry(); } + +Endpoint::Endpoint(std::string_view name) : m_name(name) { registry().push_back(this); } + +Endpoint::~Endpoint() { + auto& all = registry(); + all.erase(std::remove(all.begin(), all.end(), this), all.end()); +} + +void Endpoint::restart() { + if (!m_loop) return; + m_restartPending = true; + stop(); +} + +void Endpoint::settled() { + if (!m_restartPending) return; + m_restartPending = false; + restartNow(m_loop, m_port); +} + +// +// Server +// + +void Server::start(uv_loop_t* loop, int port) { + if (status() == Status::Running) return; + m_loop = loop; + m_port = port; + m_async = new uv_async_t(); + m_listener.start(port, loop, m_async, [this](UvFifo* fifo) { onListenerEvent(fifo); }); +} + +void Server::onListenerEvent(UvFifo* fifo) { + if (fifo) { + onConnection(IO(fifo)); + return; + } + // nullptr means the listener is finished, whether that was an orderly stop + // or a bind/listen failure. status() distinguishes them; both land here so + // there is exactly one teardown path. + if (m_async) { + uv_close(reinterpret_cast(m_async), + [](uv_handle_t* handle) { delete reinterpret_cast(handle); }); + m_async = nullptr; + } + onStopped(); + settled(); +} + +void Server::stop() { + auto listenerStatus = m_listener.status(); + if ((listenerStatus == UvFifoListener::Status::Listening) || + (listenerStatus == UvFifoListener::Status::Starting)) { + m_listener.stop(); + return; // onListenerEvent(nullptr) finishes the teardown + } + // Already down - a failed bind has torn itself down and delivered its + // nullptr already, so nothing further is going to call back. Settle here + // rather than waiting for an event that will never arrive, which is what + // would strand a pending restart. + onListenerEvent(nullptr); +} + +Status Server::status() const { + switch (m_listener.status()) { + case UvFifoListener::Status::Stopped: + return Status::Stopped; + case UvFifoListener::Status::Starting: + return Status::Starting; + case UvFifoListener::Status::Listening: + return Status::Running; + case UvFifoListener::Status::Failed: + return Status::Failed; + } + return Status::Stopped; +} + +// +// Client +// + +void Client::start(uv_loop_t* loop, std::string_view host, int port) { + m_connection.reset(); + m_fifo = nullptr; + m_loop = loop; + m_host = host; + m_port = port; + auto fifo = new UvFifo(host, port); + m_fifo = fifo; + m_connection = IO(fifo); +} + +void Client::stop() { + m_connection.reset(); + m_fifo = nullptr; + settled(); +} + +Status Client::status() const { + if (!m_fifo) return Status::Stopped; + // Order matters: a failed connect leaves the fifo both failed and closed, + // and "failed" is the answer that carries information. + if (m_fifo->failed()) return Status::Failed; + if (m_fifo->isConnecting()) return Status::Starting; + if (m_fifo->isClosed()) return Status::Stopped; + return Status::Running; +} + +const char* Client::lastError() const { return m_fifo ? m_fifo->connectError() : ""; } + +} // namespace PCSX::Network diff --git a/src/support/network.h b/src/support/network.h new file mode 100644 index 000000000..2368ba0a0 --- /dev/null +++ b/src/support/network.h @@ -0,0 +1,160 @@ +/*************************************************************************** + * Copyright (C) 2026 PCSX-Redux authors * + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + * This program is distributed in the hope that it will be useful, * + * but WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * + * GNU General Public License for more details. * + * * + * You should have received a copy of the GNU General Public License * + * along with this program; if not, write to the * + * Free Software Foundation, Inc., * + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. * + ***************************************************************************/ + +#pragma once + +#include + +#include +#include + +#include "support/file.h" +#include "support/uvfile.h" + +namespace PCSX { + +namespace Network { + +// One vocabulary for every network endpoint in the emulator, so the UI can draw +// a bullet without knowing whether it is looking at a listening socket or an +// outgoing connection. Previously each server declared its own three-state enum +// - GdbServerStatus, WebServerStatus, SIO1ServerStatus, SIO1ClientStatus, +// ATConsServer::ServerStatus - none of which agreed on spelling, none of which +// had a way to say "failed", and none of which anything ever read. +enum class Status { + Stopped, // idle, by choice + Starting, // bind or connect in flight on the uv worker thread + Running, // listening, or connected + Failed, // bind, listen, or connect failed; lastError() says why +}; + +const char* toString(Status status); + +// Common surface for anything the Network window renders a row for. +class Endpoint { + public: + Endpoint(std::string_view name); + virtual ~Endpoint(); + + Endpoint(const Endpoint&) = delete; + Endpoint& operator=(const Endpoint&) = delete; + + std::string_view name() const { return m_name; } + int port() const { return m_port; } + + virtual Status status() const = 0; + virtual const char* lastError() const = 0; + + bool isRunning() const { return status() == Status::Running; } + + // Every endpoint remembers the loop and port it was last asked to use, so + // the UI's restart button does not have to know either. + virtual void stop() = 0; + void restart(); + + // Registered endpoints, in construction order. The Network window iterates + // this rather than hard-coding a row per service, so a new endpoint shows + // up with a status bullet for free. + static const std::vector& all(); + + protected: + virtual void restartNow(uv_loop_t* loop, int port) = 0; + // Called by subclasses once a teardown has completed, so a restart that was + // requested while the endpoint was still shutting down can proceed. + void settled(); + + std::string m_name; + uv_loop_t* m_loop = nullptr; + int m_port = 0; + bool m_restartPending = false; +}; + +// A listening endpoint. Owns a UvFifoListener, which owns the socket; every +// accepted connection arrives as an IO, so subclasses never touch libuv. +// +// This replaces four hand-written copies of the same uv_tcp_init / uv_ip4_addr +// / uv_tcp_bind / uv_listen sequence. +class Server : public Endpoint { + public: + Server(std::string_view name) : Endpoint(name) {} + + void start(uv_loop_t* loop, int port); + void stop() override; + + Status status() const override; + const char* lastError() const override { return m_listener.lastError(); } + + protected: + // A client connected. The endpoint takes ownership. + virtual void onConnection(IO connection) = 0; + // The listener is finished - either an orderly stop, or a bind/listen + // failure. Check status() to tell which. Subclasses use this to drop any + // connection state they were holding. + virtual void onStopped() {} + + void restartNow(uv_loop_t* loop, int port) override { start(loop, port); } + + private: + void onListenerEvent(UvFifo* fifo); + + UvFifoListener m_listener; + // Heap allocated per start() and deleted by its own close callback. A value + // member would have to be uv_async_init'd again while the previous close was + // still in flight, which is exactly the handle-reuse hazard a restart would + // walk into. m_server inside the listener has no such problem: its status + // only reaches Stopped/Failed from inside its close callback, so by the time + // anything downstream reacts the handle is fully closed. + uv_async_t* m_async = nullptr; +}; + +// An outgoing endpoint: SIO1's client half, and anything else that dials out. +// The connection itself is a UvFifo, so status is derived from it rather than +// tracked separately - there is no second copy of the truth to go stale. +class Client : public Endpoint { + public: + Client(std::string_view name) : Endpoint(name) {} + + void start(uv_loop_t* loop, std::string_view host, int port); + void stop() override; + + Status status() const override; + const char* lastError() const override; + + std::string_view host() const { return m_host; } + + // The connection, valid once status() reads Running. Deliberately polled + // rather than delivered by callback: the connect completes on the uv worker + // thread and UvFifo exposes its progress as flags, so a "connected" event + // would be a second copy of the truth that could disagree with the first. + // Consumers that want to be woken on traffic install a notifier on it. + IO connection() { return m_connection; } + + protected: + void restartNow(uv_loop_t* loop, int port) override { start(loop, m_host, port); } + + IO m_connection; + UvFifo* m_fifo = nullptr; // non-owning view of m_connection, for status + + private: + std::string m_host; +}; + +} // namespace Network + +} // namespace PCSX diff --git a/tests/support/network.cc b/tests/support/network.cc new file mode 100644 index 000000000..1e61c6e29 --- /dev/null +++ b/tests/support/network.cc @@ -0,0 +1,220 @@ +/*************************************************************************** + * Copyright (C) 2026 PCSX-Redux authors * + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + * This program is distributed in the hope that it will be useful, * + * but WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * + * GNU General Public License for more details. * + * * + * You should have received a copy of the GNU General Public License * + * along with this program; if not, write to the * + * Free Software Foundation, Inc., * + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. * + ***************************************************************************/ + +#include "support/network.h" + +#include + +#include +#include +#include +#include + +#include "gtest/gtest.h" + +using namespace PCSX; + +namespace { + +constexpr int c_serverPort = 47841; +constexpr int c_busyPort = 47843; +constexpr int c_restartPort = 47845; +constexpr int c_deadPort = 47847; + +void pump(uv_loop_t* loop, int milliseconds = 300) { + auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(milliseconds); + while (std::chrono::steady_clock::now() < deadline) { + uv_run(loop, UV_RUN_NOWAIT); + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + } +} + +// Holds a port so a Server's bind is guaranteed to fail. Releasable mid-test, +// so the restart case can watch a Failed endpoint recover. +struct Squatter { + Squatter(uv_loop_t* loop, int port) { + uv_tcp_init(loop, &m_tcp); + struct sockaddr_in addr; + EXPECT_EQ(uv_ip4_addr("0.0.0.0", port, &addr), 0); + EXPECT_EQ(uv_tcp_bind(&m_tcp, reinterpret_cast(&addr), 0), 0); + EXPECT_EQ(uv_listen(reinterpret_cast(&m_tcp), 16, [](uv_stream_t*, int) {}), 0); + } + void release(uv_loop_t* loop) { + if (m_released) return; + m_released = true; + uv_close(reinterpret_cast(&m_tcp), [](uv_handle_t*) {}); + pump(loop); + } + ~Squatter() { + if (!m_released) uv_close(reinterpret_cast(&m_tcp), [](uv_handle_t*) {}); + } + uv_tcp_t m_tcp = {}; + bool m_released = false; +}; + +// The whole point of the base class: a concrete endpoint says what to do with a +// connection and nothing else. No uv_tcp_init, no bind, no accept, no status +// enum of its own. +class TestServer : public Network::Server { + public: + TestServer() : Network::Server("test-server") {} + std::vector> m_connections; + int m_stoppedCount = 0; + + protected: + void onConnection(IO connection) override { m_connections.push_back(connection); } + void onStopped() override { m_stoppedCount++; } +}; + +class TestClient : public Network::Client { + public: + TestClient() : Network::Client("test-client") {} +}; + +} // namespace + +TEST(NetworkServer, AcceptsAndReportsRunning) { + UvThreadOp::UvThread uvThread; + uv_loop_t loop; + uv_loop_init(&loop); + + TestServer server; + EXPECT_EQ(server.status(), Network::Status::Stopped); + + server.start(&loop, c_serverPort); + pump(&loop); + EXPECT_EQ(server.status(), Network::Status::Running); + EXPECT_TRUE(server.isRunning()); + EXPECT_STREQ(server.lastError(), ""); + EXPECT_EQ(server.port(), c_serverPort); + + IO client(new UvFifo("127.0.0.1", c_serverPort)); + pump(&loop); + ASSERT_EQ(server.m_connections.size(), 1u); + + // The connection is a plain File. That is the deletion: no per-server + // WriteRequest queue, no alloc/read trampolines, no accept boilerplate. + static const char c_payload[] = "ping"; + client->write(c_payload, sizeof(c_payload) - 1); + pump(&loop); + EXPECT_EQ(server.m_connections[0]->size(), sizeof(c_payload) - 1); + + client.reset(); + server.m_connections.clear(); + server.stop(); + pump(&loop); + EXPECT_EQ(server.status(), Network::Status::Stopped); + EXPECT_EQ(server.m_stoppedCount, 1); + + uv_run(&loop, UV_RUN_NOWAIT); + uv_loop_close(&loop); +} + +TEST(NetworkServer, BindFailureIsVisible) { + UvThreadOp::UvThread uvThread; + uv_loop_t loop; + uv_loop_init(&loop); + + Squatter squatter(&loop, c_busyPort); + + TestServer server; + server.start(&loop, c_busyPort); + pump(&loop); + + EXPECT_EQ(server.status(), Network::Status::Failed); + EXPECT_FALSE(server.isRunning()); + EXPECT_STREQ(server.lastError(), uv_strerror(UV_EADDRINUSE)); + // A failure is a teardown, so subclasses get told once and only once. + EXPECT_EQ(server.m_stoppedCount, 1); + + uv_run(&loop, UV_RUN_NOWAIT); + uv_loop_close(&loop); +} + +// The restart button's actual job: recover an endpoint that is sitting in +// Failed. This exercises the path where stop() has nothing left to close and +// must settle synchronously - if it waited for a listener callback that is +// never coming, the restart would silently never happen. +TEST(NetworkServer, RestartRecoversFromFailure) { + UvThreadOp::UvThread uvThread; + uv_loop_t loop; + uv_loop_init(&loop); + + auto squatter = std::make_unique(&loop, c_restartPort); + + TestServer server; + server.start(&loop, c_restartPort); + pump(&loop); + ASSERT_EQ(server.status(), Network::Status::Failed); + + squatter->release(&loop); + squatter.reset(); + + server.restart(); + pump(&loop); + + EXPECT_EQ(server.status(), Network::Status::Running); + EXPECT_STREQ(server.lastError(), ""); + + server.stop(); + pump(&loop); + EXPECT_EQ(server.status(), Network::Status::Stopped); + + uv_run(&loop, UV_RUN_NOWAIT); + uv_loop_close(&loop); +} + +TEST(NetworkClient, ConnectFailureIsVisible) { + UvThreadOp::UvThread uvThread; + uv_loop_t loop; + uv_loop_init(&loop); + + TestClient client; + EXPECT_EQ(client.status(), Network::Status::Stopped); + + client.start(&loop, "127.0.0.1", c_deadPort); + auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + while ((client.status() == Network::Status::Starting) && (std::chrono::steady_clock::now() < deadline)) { + pump(&loop, 20); + } + + EXPECT_EQ(client.status(), Network::Status::Failed); + EXPECT_STREQ(client.lastError(), uv_strerror(UV_ECONNREFUSED)); + + client.stop(); + EXPECT_EQ(client.status(), Network::Status::Stopped); + + uv_run(&loop, UV_RUN_NOWAIT); + uv_loop_close(&loop); +} + +// The UI iterates this instead of hard-coding a row per service, so an endpoint +// that forgets to register is an endpoint with no status bullet. +TEST(NetworkEndpoint, RegistersAndUnregisters) { + const size_t before = Network::Endpoint::all().size(); + { + TestServer server; + TestClient client; + const auto& all = Network::Endpoint::all(); + ASSERT_EQ(all.size(), before + 2); + EXPECT_EQ(all[before]->name(), "test-server"); + EXPECT_EQ(all[before + 1]->name(), "test-client"); + } + EXPECT_EQ(Network::Endpoint::all().size(), before); +} From b436c5264fad7292d5a766fffeea3b0bd6960a02 Mon Sep 17 00:00:00 2001 From: Nicolas 'Pixel' Noble Date: Tue, 28 Jul 2026 16:11:23 -0700 Subject: [PATCH 05/13] GDB server: move onto the common network endpoint. The server drops its hand-rolled bind and listen, its private three-state status enum that nothing read, its dead m_gotError member, and its accept trampolines. A connection now arrives as an IO. GdbClient loses the WriteRequest intrusive hash table and its 3-buffer scatter uv_writes along with the alloc and read trampolines and the manual close callback. A File has no scatter/gather, so a packet is framed into a single buffer and written as one Slice; GDB packets are small, and that is one allocation per packet against a hash table insert plus erase. Reads arrive through the fifo notifier rather than a poll, so there is no added latency on the stepping path. Signed-off-by: Nicolas 'Pixel' Noble --- src/core/gdb-server.cc | 127 ++++++++++++++++---------- src/core/gdb-server.h | 200 ++++++++++++----------------------------- src/gui/gui.cc | 6 +- 3 files changed, 142 insertions(+), 191 deletions(-) diff --git a/src/core/gdb-server.cc b/src/core/gdb-server.cc index b9d570a24..fed41d0b1 100644 --- a/src/core/gdb-server.cc +++ b/src/core/gdb-server.cc @@ -34,75 +34,108 @@ const char PCSX::GdbClient::toHex[] = "0123456789ABCDEF"; -PCSX::GdbServer::GdbServer() : m_listener(g_system->m_eventBus) { +PCSX::GdbServer::GdbServer() : Network::Server("GDB Server"), m_listener(g_system->m_eventBus) { m_listener.listen([this](const auto& event) { - auto& args = g_system->getArgs(); auto& settings = g_emulator->settings.get(); - if (settings.get() && (m_serverStatus != SERVER_STARTED)) { - startServer(g_system->getLoop(), settings.get()); + if (settings.get() && !isRunning()) { + start(g_system->getLoop(), settings.get()); } }); m_listener.listen([this](const auto& event) { - if (m_serverStatus == SERVER_STARTED) stopServer(); + if (isRunning()) stop(); }); } -void PCSX::GdbServer::stopServer() { - assert(m_serverStatus == SERVER_STARTED); - m_serverStatus = SERVER_STOPPING; - for (auto& client : m_clients) client.close(); - uv_close(reinterpret_cast(&m_server), closeCB); +void PCSX::GdbServer::onConnection(IO connection) { + m_clients.push_back(new GdbClient(connection, g_system->getLoop())); } -void PCSX::GdbServer::startServer(uv_loop_t* loop, int port) { - assert(m_serverStatus == SERVER_STOPPED); - - uv_tcp_init(loop, &m_server); - m_server.data = this; - - struct sockaddr_in bindAddr; - int result = uv_ip4_addr("0.0.0.0", port, &bindAddr); - if (result != 0) { - uv_close(reinterpret_cast(&m_server), closeCB); - return; +void PCSX::GdbServer::onStopped() { + // Covers an orderly stop and a failed bind alike. Iterate defensively: + // closing a client eventually deletes it, which unlinks it from this list. + while (!m_clients.empty()) { + auto client = m_clients.begin(); + client->close(); + if (!m_clients.empty() && (m_clients.begin() == client)) m_clients.erase(client); } - result = uv_tcp_bind(&m_server, reinterpret_cast(&bindAddr), 0); - if (result != 0) { - uv_close(reinterpret_cast(&m_server), closeCB); - return; - } - result = uv_listen((uv_stream_t*)&m_server, 16, onNewConnectionTrampoline); - if (result != 0) { - uv_close(reinterpret_cast(&m_server), closeCB); +} + +void PCSX::GdbClient::logOutgoing(const Slice& slice) { + if (!g_emulator->settings.get() + .get()) { return; } - m_serverStatus = SERVER_STARTED; + std::string msg(static_cast(slice.data()), slice.size()); + g_system->log(LogClass::GDB, "GDB <-- PCSX %s\n", msg.c_str()); } -void PCSX::GdbServer::closeCB(uv_handle_t* handle) { - GdbServer* self = static_cast(handle->data); - self->m_serverStatus = SERVER_STOPPED; +void PCSX::GdbClient::sendRaw(Slice&& raw) { + if (!m_connection || m_connection->isClosed()) return; + logOutgoing(raw); + m_connection->write(std::move(raw)); } -void PCSX::GdbServer::onNewConnectionTrampoline(uv_stream_t* handle, int status) { - GdbServer* self = static_cast(handle->data); - self->onNewConnection(status); +void PCSX::GdbClient::sendPacket(Slice&& payload) { + if (!m_connection || m_connection->isClosed()) return; + logOutgoing(payload); + uint8_t chksum = 0; + auto data = static_cast(payload.data()); + for (size_t i = 0; i < payload.size(); i++) chksum += data[i]; + std::string framed; + framed.reserve(payload.size() + 4); + framed += '$'; + framed.append(static_cast(payload.data()), payload.size()); + framed += '#'; + framed += toHex[chksum >> 4]; + framed += toHex[chksum & 0x0f]; + Slice out; + out.acquire(std::move(framed)); + m_connection->write(std::move(out)); } -void PCSX::GdbServer::onNewConnection(int status) { - if (status < 0) return; - GdbClient* client = new GdbClient(&m_server); - if (client->accept(&m_server)) { - m_clients.push_back(client); - } else { - delete client; +void PCSX::GdbClient::onReadable() { + uint8_t buffer[BUFFER_SIZE]; + while (m_connection && (m_status == OPEN) && (m_connection->size() > 0)) { + auto toRead = std::min(sizeof(buffer), m_connection->size()); + auto got = m_connection->read(buffer, toRead); + if (got <= 0) break; + Slice slice; + slice.borrow(buffer, got); + processData(slice); + } + // eof() is closed-and-drained, so a peer that hung up mid-packet still gets + // everything it managed to send processed before we tear the client down. + if (m_connection && m_connection->eof()) close(); +} + +void PCSX::GdbClient::close() { + if (m_status != OPEN) return; + m_status = CLOSING; + if (m_connection) { + m_connection.asA()->clearNotifier(); + m_connection.reset(); } + // Deleting the client here would free the object the async callback is + // running inside of. Hand that off to the async's own close callback, which + // uv only runs once the handle is genuinely done. + auto context = m_asyncContext; + m_asyncContext = nullptr; + if (!context) { + delete this; + return; + } + uv_close(reinterpret_cast(&context->m_async), [](uv_handle_t* handle) { + auto context = reinterpret_cast(handle); + delete context->m_client; + delete context; + }); } -PCSX::GdbClient::GdbClient(uv_tcp_t* srv) : m_listener(g_system->m_eventBus) { - m_loop = srv->loop; - uv_tcp_init(m_loop, &m_tcp); - m_tcp.data = this; +PCSX::GdbClient::GdbClient(IO connection, uv_loop_t* loop) : m_listener(g_system->m_eventBus) { + m_loop = loop; + m_connection = connection; + m_asyncContext = new AsyncContext{{}, this}; + m_connection.asA()->setNotifier(loop, &m_asyncContext->m_async, [this]() { onReadable(); }); m_listener.listen([this](const auto& event) { m_exception = false; }); m_listener.listen([this](const auto& event) { m_exception = event.exception; diff --git a/src/core/gdb-server.h b/src/core/gdb-server.h index 755efcd85..8eefa005b 100644 --- a/src/core/gdb-server.h +++ b/src/core/gdb-server.h @@ -29,62 +29,47 @@ #include "support/eventbus.h" #include "support/hashtable.h" #include "support/list.h" +#include "support/network.h" #include "support/slice.h" namespace PCSX { class GdbClient : public Intrusive::List::Node { public: - GdbClient(uv_tcp_t* srv); - ~GdbClient() { - assert(m_requests.size() == 0); - m_breakpoints.destroyAll(); - } + GdbClient(IO connection, uv_loop_t* loop); + ~GdbClient() { m_breakpoints.destroyAll(); } typedef Intrusive::List ListType; - bool accept(uv_tcp_t* srv) { - assert(m_status == CLOSED); - if (uv_accept(reinterpret_cast(srv), reinterpret_cast(&m_tcp)) == 0) { - uv_read_start(reinterpret_cast(&m_tcp), allocTrampoline, readTrampoline); - m_status = OPEN; - } - return m_status == OPEN; - } - void close() { - if (m_status != OPEN) return; - m_status = CLOSING; - uv_close(reinterpret_cast(&m_tcp), closeCB); - } + void close(); private: void write(const Slice& slice) { - auto* req = new WriteRequest(); - req->m_slice = slice; - req->enqueue(this); + Slice payload; + payload.copy(slice.data(), slice.size()); + sendPacket(std::move(payload)); } void write(const std::string& msg) { - auto* req = new WriteRequest(); assert(msg.size() <= std::numeric_limits::max()); - req->m_slice.copy(msg); - req->enqueue(this); + Slice payload; + payload.copy(msg); + sendPacket(std::move(payload)); } void write(std::string&& msg) { - auto* req = new WriteRequest(); assert(msg.size() <= std::numeric_limits::max()); - req->m_slice.acquire(std::move(msg)); - req->enqueue(this); + Slice payload; + payload.acquire(std::move(msg)); + sendPacket(std::move(payload)); } template void write(const char (&str)[L]) { - auto* req = new WriteRequest(); static_assert((L - 1) <= std::numeric_limits::max()); - req->m_slice.borrow(str, L - 1); - req->enqueue(this); + Slice payload; + payload.borrow(str, L - 1); + sendPacket(std::move(payload)); } void writef(const char* fmt, ...) { va_list a; va_start(a, fmt); - auto* req = new WriteRequest(); size_t len; char* msg; #ifdef _WIN32 @@ -94,122 +79,68 @@ class GdbClient : public Intrusive::List::Node { #else len = vasprintf(&msg, fmt, a); #endif - req->m_slice.acquire(msg, len); - req->enqueue(this); + Slice payload; + payload.acquire(msg, len); + sendPacket(std::move(payload)); va_end(a); } void writePaged(const std::string& out, const std::string& cursorStr); void writeEscaped(const std::string& out); void sendAck() { - auto* req = new WriteRequest(); - req->m_slice.copy("+", 1); - req->enqueueRaw(this); + Slice raw; + raw.copy("+", 1); + sendRaw(std::move(raw)); } void startStream() { m_crc = 0; - auto* req = new WriteRequest(); - req->m_slice.copy("$", 1); - req->enqueueRaw(this); + Slice raw; + raw.copy("$", 1); + sendRaw(std::move(raw)); } void stream(const std::string& data) { for (int i = 0; i < data.length(); i++) { m_crc += data[i]; } - auto* req = new WriteRequest(); - req->m_slice.copy(data.data(), data.size()); - req->enqueueRaw(this); + Slice raw; + raw.copy(data.data(), data.size()); + sendRaw(std::move(raw)); } void stopStream() { - auto* req = new WriteRequest(); char end[3] = {'#'}; end[1] = toHex[m_crc >> 4]; end[2] = toHex[m_crc & 0x0f]; - req->m_slice.copy(end, 3); - req->enqueueRaw(this); + Slice raw; + raw.copy(end, 3); + sendRaw(std::move(raw)); } static const char toHex[]; - struct WriteRequest : public Intrusive::HashTable::Node { - void enqueue(GdbClient* client) { - if (g_emulator->settings.get() - .get()) { - std::string msg((const char*)m_slice.data(), m_slice.size()); - g_system->log(LogClass::GDB, "GDB <-- PCSX %s\n", msg.c_str()); - } - m_bufs[0].base = &m_before; - m_bufs[0].len = 1; - m_bufs[1].base = static_cast(const_cast(m_slice.data())); - m_bufs[1].len = m_slice.size(); - m_bufs[2].base = m_after; - m_bufs[2].len = 3; - uint8_t chksum = 0; - auto data = m_bufs[1].base; - auto len = m_bufs[1].len; - for (int i = 0; i < len; i++) { - chksum += *data++; - } - m_after[1] = toHex[chksum >> 4]; - m_after[2] = toHex[chksum & 0x0f]; - client->m_requests.insert(reinterpret_cast(&m_req), this); - uv_write(&m_req, reinterpret_cast(&client->m_tcp), m_bufs, 3, writeCB); - } - void enqueueRaw(GdbClient* client) { - if (g_emulator->settings.get() - .get()) { - std::string msg((const char*)m_slice.data(), m_slice.size()); - g_system->log(LogClass::GDB, "GDB <-- PCSX %s\n", msg.c_str()); - } - m_bufs[0].base = static_cast(const_cast(m_slice.data())); - m_bufs[0].len = m_slice.size(); - client->m_requests.insert(reinterpret_cast(&m_req), this); - uv_write(&m_req, reinterpret_cast(&client->m_tcp), m_bufs, 1, writeCB); - } - static void writeCB(uv_write_t* request, int status) { - GdbClient* client = static_cast(request->handle->data); - auto self = client->m_requests.find(reinterpret_cast(request)); - delete &*self; - if (status != 0) client->close(); - } - uv_write_t m_req; - char m_before = '$'; - char m_after[3] = {'#'}; - uv_buf_t m_bufs[3]; - Slice m_slice; - }; - friend struct WriteRequest; - Intrusive::HashTable m_requests; + + // Framing and transport. Previously this was a WriteRequest intrusive hash + // table doing 3-buffer scatter uv_writes, duplicated verbatim in the web + // server. A File has no scatter/gather, so a packet is framed into one + // buffer and handed over as a single Slice - GDB packets are small, and it + // is one allocation per packet against a hash table insert plus erase. + void sendPacket(Slice&& payload); // wraps in $...#XX + void sendRaw(Slice&& raw); // as-is, for acks and streamed chunks + void logOutgoing(const Slice& slice); + static constexpr size_t BUFFER_SIZE = 256; - static void allocTrampoline(uv_handle_t* handle, size_t suggestedSize, uv_buf_t* buf) { - GdbClient* client = static_cast(handle->data); - client->alloc(suggestedSize, buf); - } - void alloc(size_t suggestedSize, uv_buf_t* buf) { - assert(!m_allocated); - m_allocated = true; - buf->base = m_buffer; - buf->len = sizeof(m_buffer); - } - static void readTrampoline(uv_stream_t* stream, ssize_t nread, const uv_buf_t* buf) { - GdbClient* client = static_cast(stream->data); - client->read(nread, buf); - } - void read(ssize_t nread, const uv_buf_t* buf) { - m_allocated = false; - if (nread <= 0) { - close(); - return; - } - Slice slice; - slice.borrow(m_buffer, nread); - processData(slice); - } - static void closeCB(uv_handle_t* handle) { - GdbClient* client = static_cast(handle->data); - delete client; - } + // Woken by the fifo's notifier; drains whatever arrived into processData. + void onReadable(); + + // The async lives here rather than in the client so that the close callback + // can find its way back to the client after uv is done with the handle - + // UvFifo::setNotifier owns the handle's data pointer. + struct AsyncContext { + uv_async_t m_async; + GdbClient* m_client; + }; + AsyncContext* m_asyncContext = nullptr; + void processData(const Slice& slice); void processCommand(); void processMonitorCommand(const std::string&); @@ -220,11 +151,9 @@ class GdbClient : public Intrusive::List::Node { void setOneRegister(int n, uint32_t value); static std::string dumpValue(uint32_t value); - uv_tcp_t m_tcp; - enum { CLOSED, OPEN, CLOSING } m_status = CLOSED; + IO m_connection; + enum { OPEN, CLOSING } m_status = OPEN; - char m_buffer[BUFFER_SIZE]; - bool m_allocated = false; enum { WAIT_FOR_ACK, WAIT_FOR_DOLLAR, @@ -248,28 +177,17 @@ class GdbClient : public Intrusive::List::Node { Debug::BreakpointUserListType m_breakpoints; }; -class GdbServer { +class GdbServer : public Network::Server { public: GdbServer(); - enum GdbServerStatus { - SERVER_STOPPED, - SERVER_STOPPING, - SERVER_STARTED, - }; - GdbServerStatus getServerStatus() { return m_serverStatus; } - void startServer(uv_loop_t* loop, int port = 3333); - void stopServer(); + protected: + void onConnection(IO connection) override; + void onStopped() override; private: - static void onNewConnectionTrampoline(uv_stream_t* server, int status); - void onNewConnection(int status); - static void closeCB(uv_handle_t* handle); - GdbServerStatus m_serverStatus = SERVER_STOPPED; - uv_tcp_t m_server; GdbClient::ListType m_clients; EventBus::Listener m_listener; - std::string m_gotError; }; } // namespace PCSX diff --git a/src/gui/gui.cc b/src/gui/gui.cc index 18e98898c..29187f3ce 100644 --- a/src/gui/gui.cc +++ b/src/gui/gui.cc @@ -2227,10 +2227,10 @@ can slow down emulation to a noticeable extent.)")); if (ImGui::Checkbox(_("Enable GDB Server"), &debugSettings.get().value)) { changed = true; if (debugSettings.get()) { - g_emulator->m_gdbServer->startServer(g_system->getLoop(), - debugSettings.get()); + g_emulator->m_gdbServer->start(g_system->getLoop(), + debugSettings.get()); } else { - g_emulator->m_gdbServer->stopServer(); + g_emulator->m_gdbServer->stop(); } } ImGuiHelpers::ShowHelpMarker(_(R"(This will activate a gdb-server that you can From 16224beecafd0e3aee2262b9b4a90d17755cf147 Mon Sep 17 00:00:00 2001 From: Nicolas 'Pixel' Noble Date: Tue, 28 Jul 2026 16:22:18 -0700 Subject: [PATCH 06/13] UvFifo: flush pending writes before closing. Writes are queued to the uv worker thread, so returning from write() and the bytes leaving the machine are different moments, and uv_close cancels whatever is still queued. Closing right after writing therefore truncates the tail: with a peer that is not draining, a 64MB write followed by an immediate close delivered 2634240 bytes. Hand the teardown to whichever write completes last. The socket and the bookkeeping move into a control block held by both the fifo and every write in flight, so a fifo destroyed while writes are still queued cannot pull the state out from under the worker thread - the write path must never capture the fifo itself for that reason. Signed-off-by: Nicolas 'Pixel' Noble --- src/support/uvfile.cc | 79 ++++++++++++++++++++++------ src/support/uvfile.h | 23 ++++++++ tests/support/uvfifolistener.cc | 93 +++++++++++++++++++++++++++++++++ 3 files changed, 179 insertions(+), 16 deletions(-) diff --git a/src/support/uvfile.cc b/src/support/uvfile.cc index 543e15012..32bf87a8c 100644 --- a/src/support/uvfile.cc +++ b/src/support/uvfile.cc @@ -778,6 +778,7 @@ void PCSX::UvFile::cacheCallbackSetup(std::function &&callbackDone, uv_l PCSX::UvFifo::UvFifo(uv_tcp_t *tcp) : File(File::FileType::RW_STREAM) { tcp->data = this; m_tcp = tcp; + m_control->m_tcp = tcp; startRead(tcp); } @@ -788,6 +789,7 @@ PCSX::UvFifo::UvFifo(const std::string_view address, unsigned port) : File(File: uv_tcp_t *tcp = new uv_tcp_t(); tcp->data = this; m_tcp = tcp; + m_control->m_tcp = tcp; request([this, host = std::string(address), port](auto loop) { uv_tcp_init(loop, m_tcp); struct sockaddr_in connectAddr; @@ -878,14 +880,31 @@ void PCSX::UvFifo::setNotifier(uv_loop_t *loop, uv_async_t *async, std::function if ((m_size.load() > 0) || m_closed.load()) notify(); } +void PCSX::UvFifo::Control::closeNow() { + auto tcp = m_tcp; + m_tcp = nullptr; + if (!tcp) return; + uv_close(reinterpret_cast(tcp), [](uv_handle_t *handle) { + auto tcp = reinterpret_cast(handle); + delete tcp; + }); +} + +void PCSX::UvFifo::Control::writeCompleted() { + auto remaining = m_pending.fetch_sub(1, std::memory_order_acq_rel) - 1; + if (m_closeRequested && (remaining == 0)) closeNow(); +} + void PCSX::UvFifo::closeInternal() { m_closed.store(true); - request([tcp = m_tcp](uv_loop_t *loop) { - if (!tcp) return; - uv_close(reinterpret_cast(tcp), [](uv_handle_t *handle) { - auto tcp = reinterpret_cast(handle); - delete tcp; - }); + request([control = m_control](uv_loop_t *loop) { + // Graceful: uv_close cancels pending uv_writes, so tearing down here + // while writes are still in flight truncates them - for an HTTP + // response that is a silently short reply. Hand the teardown to + // whichever write finishes last instead. m_closeRequested and m_tcp are + // touched only on the worker thread, so the two cannot race. + control->m_closeRequested = true; + if (control->m_pending.load(std::memory_order_acquire) == 0) control->closeNow(); }); } @@ -919,18 +938,32 @@ ssize_t PCSX::UvFifo::write(const void *src, size_t size) { uv_buf_t buf; uv_write_t req; Slice slice; + std::shared_ptr control; }; auto info = new Info(); info->req.data = info; info->slice.copy(src, size); info->buf.base = reinterpret_castbuf.base)>(const_cast(info->slice.data())); info->buf.len = size; - request([info, tcp = m_tcp](auto loop) { - info->buf.base = reinterpret_castbuf.base)>(const_cast(info->slice.data())); - uv_write(&info->req, reinterpret_cast(tcp), &info->buf, 1, [](uv_write_t *req, int status) { - auto info = reinterpret_cast(req->data); + m_control->m_pending.fetch_add(1, std::memory_order_acq_rel); + // Capture the control block, never `this`: the fifo may be destroyed while + // this write is still queued, and the worker thread must still be able to + // finish and account for it. + request([control = m_control, info](auto loop) { + if (!control->m_tcp) { delete info; - }); + control->writeCompleted(); + return; + } + info->control = control; + info->buf.base = reinterpret_castbuf.base)>(const_cast(info->slice.data())); + uv_write(&info->req, reinterpret_cast(control->m_tcp), &info->buf, 1, + [](uv_write_t *req, int status) { + auto info = reinterpret_cast(req->data); + auto control = info->control; + delete info; + control->writeCompleted(); + }); }); return size; } @@ -940,18 +973,32 @@ void PCSX::UvFifo::write(Slice &&slice) { uv_buf_t buf; uv_write_t req; Slice slice; + std::shared_ptr control; }; auto size = slice.size(); auto info = new Info(); info->req.data = info; info->buf.len = size; info->slice = std::move(slice); - request([info, tcp = m_tcp](auto loop) { - info->buf.base = reinterpret_castbuf.base)>(const_cast(info->slice.data())); - uv_write(&info->req, reinterpret_cast(tcp), &info->buf, 1, [](uv_write_t *req, int status) { - auto info = reinterpret_cast(req->data); + m_control->m_pending.fetch_add(1, std::memory_order_acq_rel); + // Capture the control block, never `this`: the fifo may be destroyed while + // this write is still queued, and the worker thread must still be able to + // finish and account for it. + request([control = m_control, info](auto loop) { + if (!control->m_tcp) { delete info; - }); + control->writeCompleted(); + return; + } + info->control = control; + info->buf.base = reinterpret_castbuf.base)>(const_cast(info->slice.data())); + uv_write(&info->req, reinterpret_cast(control->m_tcp), &info->buf, 1, + [](uv_write_t *req, int status) { + auto info = reinterpret_cast(req->data); + auto control = info->control; + delete info; + control->writeCompleted(); + }); }); } diff --git a/src/support/uvfile.h b/src/support/uvfile.h index fed9c403f..c9040b471 100644 --- a/src/support/uvfile.h +++ b/src/support/uvfile.h @@ -31,6 +31,7 @@ SOFTWARE. #include #include #include +#include #include #include @@ -267,6 +268,15 @@ class UvFifo : public File, public UvThreadOp { void setNotifier(uv_loop_t* loop, uv_async_t* async, std::function&& cb); void clearNotifier() { m_notifyAsync.store(nullptr, std::memory_order_release); } + // Writes are queued to the uv worker thread, so "I called write()" and "the + // bytes left the machine" are different moments. Closing between the two + // cancels the pending uv_writes and truncates whatever was in flight, which + // for an HTTP response is a silently short reply. close() is therefore + // graceful: the socket is torn down by the last write to complete, not by + // the caller. Exposed for consumers that want to know, e.g. to hold a + // connection open until a response has drained. + size_t pendingWrites() const { return m_control->m_pending.load(std::memory_order_acquire); } + private: virtual void closeInternal() final override; UvFifo(uv_tcp_t*); @@ -289,6 +299,19 @@ class UvFifo : public File, public UvThreadOp { Slice m_slice; size_t m_currentPtr = 0; std::atomic m_notifyAsync = nullptr; + + // The socket and the graceful-close bookkeeping outlive the UvFifo on + // purpose. An in-flight uv_write holds a reference, so a fifo destroyed + // while writes are still queued cannot pull the state out from under the + // worker thread - which is why the write path must never capture `this`. + struct Control { + std::atomic m_pending = 0; + bool m_closeRequested = false; // worker thread only + uv_tcp_t* m_tcp = nullptr; // worker thread only, after construction + void closeNow(); + void writeCompleted(); + }; + std::shared_ptr m_control = std::make_shared(); std::function m_notifyCb; friend class UvFifoListener; }; diff --git a/tests/support/uvfifolistener.cc b/tests/support/uvfifolistener.cc index 586c7d21a..b68e58c22 100644 --- a/tests/support/uvfifolistener.cc +++ b/tests/support/uvfifolistener.cc @@ -21,6 +21,7 @@ #include #include +#include #include "gtest/gtest.h" #include "support/uvfile.h" @@ -35,6 +36,7 @@ constexpr unsigned c_squattedPort = 47821; constexpr unsigned c_freePort = 47823; constexpr unsigned c_notifyPort = 47825; constexpr unsigned c_deadPort = 47827; +constexpr unsigned c_flushPort = 47829; // Pump the caller-side loop for a while, giving the uv worker thread time to // service the queued request() and hand anything back through the async. @@ -219,3 +221,94 @@ TEST(UvFifo, FailedConnectIsActionable) { EXPECT_EQ(fifo->connectErrorCode(), UV_ECONNREFUSED); EXPECT_STREQ(fifo->connectError(), uv_strerror(UV_ECONNREFUSED)); } + +// Writes are queued to the worker thread, so "write() returned" and "the bytes +// left the machine" are different moments. uv_close cancels pending uv_writes, +// so closing between the two truncates the tail - for an HTTP response that is +// a silently short reply, and it is why the web server carried its own +// m_closeScheduled / m_requests.size() bookkeeping. close() is graceful now so +// consumers do not each have to reinvent that. +// +// The peer here deliberately does NOT read while the write is in flight. An +// earlier version of this test used two UvFifos, which share the worker thread, +// so the receiver drained as fast as the sender wrote and nothing was ever +// pending at close time - that version passed with the graceful close removed, +// i.e. it measured nothing. A silent peer fills the socket buffers and forces +// the writes to stay queued, which is the only state in which this is a test. +TEST(UvFifo, CloseFlushesPendingWrites) { + UvThreadOp::UvThread uvThread; + uv_loop_t loop; + uv_loop_init(&loop); + + UvFifo* accepted = nullptr; + UvFifoListener listener; + uv_async_t listenerAsync = {}; + listener.start(c_flushPort, &loop, &listenerAsync, [&accepted](UvFifo* fifo) { + if (fifo) accepted = fifo; + }); + pump(&loop); + ASSERT_EQ(listener.status(), UvFifoListener::Status::Listening); + + // A raw peer on the test loop, with no uv_read_start: it accepts the + // connection and then stays silent. + struct Peer { + uv_tcp_t m_tcp = {}; + size_t m_received = 0; + bool m_eof = false; + bool m_reading = false; + } peer; + uv_tcp_init(&loop, &peer.m_tcp); + peer.m_tcp.data = &peer; + struct sockaddr_in target; + ASSERT_EQ(uv_ip4_addr("127.0.0.1", c_flushPort, &target), 0); + uv_connect_t connectReq; + ASSERT_EQ(uv_tcp_connect(&connectReq, &peer.m_tcp, reinterpret_cast(&target), + [](uv_connect_t*, int status) { EXPECT_EQ(status, 0); }), + 0); + pump(&loop); + ASSERT_NE(accepted, nullptr); + IO serverSide(accepted); + + // Big enough that it cannot possibly fit in the socket buffers, so the tail + // is genuinely still queued when close() lands. + constexpr size_t c_payloadSize = 64 * 1024 * 1024; + std::string payload(c_payloadSize, 'x'); + serverSide->write(payload.data(), payload.size()); + ASSERT_GT(accepted->pendingWrites(), 0u); + // No drain wait: close straight away, the way a handler that has finished + // writing its response does. + serverSide.reset(); + + // Only now start reading. + peer.m_reading = true; + uv_read_start( + reinterpret_cast(&peer.m_tcp), + [](uv_handle_t*, size_t suggested, uv_buf_t* buf) { + buf->base = static_cast(malloc(suggested)); + buf->len = suggested; + }, + [](uv_stream_t* stream, ssize_t nread, const uv_buf_t* buf) { + auto self = static_cast(stream->data); + if (nread > 0) { + self->m_received += nread; + } else if (nread < 0) { + self->m_eof = true; + } + free(buf->base); + }); + + auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(30); + while (!peer.m_eof && (std::chrono::steady_clock::now() < deadline)) { + pump(&loop, 50); + } + + EXPECT_TRUE(peer.m_eof); + EXPECT_EQ(peer.m_received, c_payloadSize); + + uv_close(reinterpret_cast(&peer.m_tcp), [](uv_handle_t*) {}); + listener.stop(); + pump(&loop); + uv_close(reinterpret_cast(&listenerAsync), [](uv_handle_t*) {}); + pump(&loop); + uv_loop_close(&loop); +} From a4521efaf67fb96f27daf63d633327fc3af8d814 Mon Sep 17 00:00:00 2001 From: Nicolas 'Pixel' Noble Date: Tue, 28 Jul 2026 16:36:23 -0700 Subject: [PATCH 07/13] Build test objects under objs/ like everything else. Test objects were compiled in place next to their sources by make's built-in suffix rule, which meant they had no header dependency tracking and were not removed by the clean target. Editing a header that a test includes therefore left the old object in place and linked it, and if the header changed a class layout the result was heap corruption at runtime rather than a build error. Route them through the same objs/ pattern rule as the rest of the tree so they pick up the existing dep generation, and stop littering tests/ with build output. Signed-off-by: Nicolas 'Pixel' Noble --- Makefile | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 96eb4dc1b..b09121630 100644 --- a/Makefile +++ b/Makefile @@ -222,11 +222,12 @@ VIXL_OBJECTS := $(addprefix objs/$(BUILD)/,$(patsubst %.cc,%.o,$(filter %.cc,$(V $(IMGUI_OBJECTS): EXTRA_CPPFLAGS := $(IMGUI_CPPFLAGS) TESTS_SRC := $(call rwildcard,tests/,*.cc) -TESTS := $(patsubst %.cc,%,$(TESTS_SRC)) +TESTS_OBJECTS := $(addprefix objs/$(BUILD)/,$(patsubst %.cc,%.o,$(TESTS_SRC))) DEPS += $(addprefix deps/$(BUILD)/,$(patsubst %.c,%.dep,$(filter %.c,$(SRCS)))) DEPS += $(addprefix deps/$(BUILD)/,$(patsubst %.cc,%.dep,$(filter %.cc,$(SRCS)))) DEPS += $(addprefix deps/$(BUILD)/,$(patsubst %.cpp,%.dep,$(filter %.cpp,$(SRCS)))) +DEPS += $(addprefix deps/$(BUILD)/,$(patsubst %.cc,%.dep,$(TESTS_SRC))) CP ?= cp MKDIRP ?= mkdir -p @@ -329,7 +330,7 @@ objs/$(BUILD)/gtest_main.o: third_party/googletest/googletest/src/gtest_main.cc $(CXX) -O3 -g $(CXXFLAGS) -Ithird_party/googletest/googletest -Ithird_party/googletest/googletest/include -c third_party/googletest/googletest/src/gtest_main.cc -o objs/$(BUILD)/gtest_main.o clean: - rm -f $(OBJECTS) $(TOOLS) $(TARGET) bins/$(BUILD)/$(TARGET) $(addprefix bins/$(BUILD)/,$(TOOLS)) $(DEPS) objs/$(BUILD)/gtest-all.o objs/$(BUILD)/gtest_main.o + rm -f $(OBJECTS) $(TESTS_OBJECTS) $(TOOLS) $(TARGET) bins/$(BUILD)/$(TARGET) $(addprefix bins/$(BUILD)/,$(TOOLS)) $(DEPS) objs/$(BUILD)/gtest-all.o objs/$(BUILD)/gtest_main.o $(MAKE) -C third_party/luajit clean MACOSX_DEPLOYMENT_TARGET=$(MACOS_MIN_VERSION) cleanall: @@ -354,9 +355,9 @@ regen-i18n: rm pcsx-src-list.txt $(foreach l,$(LOCALES),$(call msgmerge,$(l))) -bins/$(BUILD)/pcsx-redux-tests: $(foreach t,$(TESTS),$(t).o) $(NONMAIN_OBJECTS) $(LIBS) objs/$(BUILD)/gtest-all.o objs/$(BUILD)/gtest_main.o +bins/$(BUILD)/pcsx-redux-tests: $(TESTS_OBJECTS) $(NONMAIN_OBJECTS) $(LIBS) objs/$(BUILD)/gtest-all.o objs/$(BUILD)/gtest_main.o @$(MKDIRP) $(dir $@) - $(LD) -o bins/$(BUILD)/pcsx-redux-tests $(NONMAIN_OBJECTS) $(LIBS) objs/$(BUILD)/gtest-all.o objs/$(BUILD)/gtest_main.o $(foreach t,$(TESTS),$(t).o) -Ithird_party/googletest/googletest/include $(LDFLAGS) + $(LD) -o bins/$(BUILD)/pcsx-redux-tests $(NONMAIN_OBJECTS) $(LIBS) objs/$(BUILD)/gtest-all.o objs/$(BUILD)/gtest_main.o $(TESTS_OBJECTS) -Ithird_party/googletest/googletest/include $(LDFLAGS) pcsx-redux-tests: check_submodules bins/$(BUILD)/pcsx-redux-tests $(CP) bins/$(BUILD)/pcsx-redux-tests pcsx-redux-tests From b43f5750acf6916d61ce7c8e75f2c8bb836cb67a Mon Sep 17 00:00:00 2001 From: Nicolas 'Pixel' Noble Date: Tue, 28 Jul 2026 16:42:04 -0700 Subject: [PATCH 08/13] Web server: move onto the common network endpoint. Same treatment as the GDB server. The server drops its hand-rolled bind and listen, its private status enum, its dead m_gotError member and its accept path; connections arrive as IO. WebClientImpl loses the WriteRequest hash table, the alloc and read trampolines and the manual close callback. It also loses m_closeScheduled entirely: that existed to hold a connection open until every queued write had completed, because closing earlier truncated the response. The transport flushes on close now, so scheduleClose is just close. Verified against a running server: a 404 frames correctly, and the vram and ram endpoints return their full 1MB and 2MB rather than whatever fit in the socket buffer. Signed-off-by: Nicolas 'Pixel' Noble --- src/core/web-server.cc | 189 ++++++++++++++--------------------------- src/core/web-server.h | 24 ++---- src/gui/gui.cc | 6 +- 3 files changed, 72 insertions(+), 147 deletions(-) diff --git a/src/core/web-server.cc b/src/core/web-server.cc index 292abb358..edf5ba6f5 100644 --- a/src/core/web-server.cc +++ b/src/core/web-server.cc @@ -796,7 +796,7 @@ void PCSX::WebExecutor::write200(PCSX::WebClient* client, const nlohmann::json& client->write(std::move(message)); } -PCSX::WebServer::WebServer() : m_listener(g_system->m_eventBus) { +PCSX::WebServer::WebServer() : Network::Server("Web Server"), m_listener(g_system->m_eventBus) { m_executors.push_back(new VramExecutor()); m_executors.push_back(new RamExecutor()); m_executors.push_back(new AssemblyExecutor()); @@ -808,84 +808,38 @@ PCSX::WebServer::WebServer() : m_listener(g_system->m_eventBus) { m_executors.push_back(new ScreenExecutor()); m_listener.listen([this](const auto& event) { auto& debugSettings = g_emulator->settings.get(); - if (debugSettings.get() && (m_serverStatus != SERVER_STARTED)) { - startServer(g_system->getLoop(), debugSettings.get()); + if (debugSettings.get() && !isRunning()) { + start(g_system->getLoop(), debugSettings.get()); } }); m_listener.listen([this](const auto& event) { - if (m_serverStatus == SERVER_STARTED) stopServer(); + if (isRunning()) stop(); }); } -void PCSX::WebServer::stopServer() { - assert(m_serverStatus == SERVER_STARTED); - m_serverStatus = SERVER_STOPPING; - for (auto& client : m_clients) client.close(); - uv_close(reinterpret_cast(&m_server), closeCB); -} - -void PCSX::WebServer::startServer(uv_loop_t* loop, int port) { - assert(m_serverStatus == SERVER_STOPPED); - m_loop = loop; - uv_tcp_init(loop, &m_server); - m_server.data = this; - - struct sockaddr_in bindAddr; - int result = uv_ip4_addr("0.0.0.0", port, &bindAddr); - if (result != 0) { - uv_close(reinterpret_cast(&m_server), closeCB); - return; - } - result = uv_tcp_bind(&m_server, reinterpret_cast(&bindAddr), 0); - if (result != 0) { - uv_close(reinterpret_cast(&m_server), closeCB); - return; +void PCSX::WebServer::onStopped() { + // Covers an orderly stop and a failed bind alike. Closing a client + // eventually deletes it, which unlinks it from this list. + while (!m_clients.empty()) { + auto client = m_clients.begin(); + client->close(); + if (!m_clients.empty() && (m_clients.begin() == client)) m_clients.erase(client); } - result = uv_listen((uv_stream_t*)&m_server, 16, [](uv_stream_t* handle, int status) { - WebServer* self = static_cast(handle->data); - self->onNewConnection(status); - }); - if (result != 0) { - uv_close(reinterpret_cast(&m_server), closeCB); - return; - } - m_serverStatus = SERVER_STARTED; -} - -void PCSX::WebServer::closeCB(uv_handle_t* handle) { - WebServer* self = static_cast(handle->data); - self->m_serverStatus = SERVER_STOPPED; } struct PCSX::WebClient::WebClientImpl { - struct WriteRequest : public Intrusive::HashTable::Node { - WriteRequest() {} - WriteRequest(Slice&& slice) : m_slice(std::move(slice)) {} - void enqueue(WebClientImpl* client) { - if (client->m_closeScheduled) { - delete this; - return; - } - m_buf.base = static_cast(const_cast(m_slice.data())); - m_buf.len = m_slice.size(); - client->m_requests.insert(reinterpret_cast(&m_req), this); - uv_write(&m_req, reinterpret_cast(&client->m_tcp), &m_buf, 1, writeCB); - } - static void writeCB(uv_write_t* request, int status) { - WebClientImpl* client = static_cast(request->handle->data); - auto self = client->m_requests.find(reinterpret_cast(request)); - delete &*self; - if ((status != 0) || (client->m_closeScheduled && (client->m_requests.size() == 0))) client->close(); - } - uv_buf_t m_buf; - uv_write_t m_req; - Slice m_slice; + // The async lives here rather than in the client so the close callback can + // find its way back after uv is done with the handle - UvFifo::setNotifier + // owns the handle's data pointer. + struct AsyncContext { + uv_async_t m_async; + WebClientImpl* m_impl; }; - Intrusive::HashTable m_requests; - WebClientImpl(WebServer* server, WebClient* parent) : m_server(server), m_parent(parent) { - uv_tcp_init(server->m_loop, &m_tcp); - m_tcp.data = this; + WebClientImpl(WebServer* server, WebClient* parent, IO connection, uv_loop_t* loop) + : m_server(server), m_parent(parent), m_connection(connection) { + m_asyncContext = new AsyncContext{{}, this}; + m_connection.asA()->setNotifier(loop, &m_asyncContext->m_async, [this]() { onReadable(); }); llhttp_settings_init(&m_httpParserSettings); m_httpParserSettings.on_message_begin = [](auto* parser) { return static_cast(parser->data)->onMessageBegin(); @@ -933,24 +887,27 @@ struct PCSX::WebClient::WebClientImpl { void close() { if (m_status != OPEN) return; m_status = CLOSING; - uv_close(reinterpret_cast(&m_tcp), closeCB); - } - bool accept(uv_tcp_t* srv) { - assert(m_status == CLOSED); - if (uv_accept(reinterpret_cast(srv), reinterpret_cast(&m_tcp)) == 0) { - uv_read_start( - reinterpret_cast(&m_tcp), - [](uv_handle_t* handle, size_t suggestedSize, uv_buf_t* buf) { - WebClientImpl* client = static_cast(handle->data); - client->alloc(suggestedSize, buf); - }, - [](uv_stream_t* stream, ssize_t nread, const uv_buf_t* buf) { - WebClientImpl* client = static_cast(stream->data); - client->read(nread, buf); - }); - m_status = OPEN; + // Dropping the connection is enough: UvFifo::close() flushes whatever + // is still queued before tearing the socket down, so a response cannot + // be truncated. That guarantee is what retired the m_closeScheduled and + // m_requests.size() bookkeeping this class used to carry. + if (m_connection) { + m_connection.asA()->clearNotifier(); + m_connection.reset(); } - return m_status == OPEN; + auto context = m_asyncContext; + m_asyncContext = nullptr; + if (!context) { + delete m_parent; + return; + } + // Deleting from here would free the object whose callback we are inside + // of; let the async's own close callback do it. + uv_close(reinterpret_cast(&context->m_async), [](uv_handle_t* handle) { + auto context = reinterpret_cast(handle); + delete context->m_impl->m_parent; + delete context; + }); } void onEOF() { @@ -1100,25 +1057,18 @@ struct PCSX::WebClient::WebClientImpl { } int onChunkHeader() { return 0; } int onChunkComplete() { return 0; } - void alloc(size_t suggestedSize, uv_buf_t* buf) { - assert(!m_allocated); - m_allocated = true; - buf->base = m_buffer; - buf->len = sizeof(m_buffer); - } - void read(ssize_t nread, const uv_buf_t* buf) { - m_allocated = false; - if (nread <= 0) { - onEOF(); - return; + void onReadable() { + uint8_t buffer[BUFFER_SIZE]; + while (m_connection && (m_status == OPEN) && (m_connection->size() > 0)) { + auto got = m_connection->read(buffer, std::min(sizeof(buffer), m_connection->size())); + if (got <= 0) break; + Slice slice; + slice.borrow(buffer, got); + processData(slice); } - Slice slice; - slice.borrow(m_buffer, nread); - processData(slice); - } - static void closeCB(uv_handle_t* handle) { - WebClientImpl* client = static_cast(handle->data); - delete client->m_parent; + // eof() is closed-and-drained, so a request that arrived alongside the + // peer hanging up still gets parsed before the connection is finished. + if (m_connection && (m_status == OPEN) && m_connection->eof()) onEOF(); } void processData(const Slice& slice) { const char* ptr = reinterpret_cast(slice.data()); @@ -1142,8 +1092,8 @@ struct PCSX::WebClient::WebClientImpl { } void write(Slice&& slice) { - auto* req = new WriteRequest(std::move(slice)); - req->enqueue(this); + if (!m_connection || (m_status != OPEN)) return; + m_connection->write(std::move(slice)); } void write(std::string&& str) { @@ -1182,20 +1132,14 @@ struct PCSX::WebClient::WebClientImpl { scheduleClose(); return 0; } - void scheduleClose() { - if (m_requests.size() == 0) { - close(); - } else { - m_closeScheduled = true; - } - } + // The transport flushes on close now, so there is nothing left to wait for. + void scheduleClose() { close(); } WebServer* m_server; - uv_tcp_t m_tcp; + IO m_connection; + AsyncContext* m_asyncContext = nullptr; static constexpr size_t BUFFER_SIZE = 256; - char m_buffer[BUFFER_SIZE]; - bool m_allocated = false; - enum { CLOSED, OPEN, CLOSING } m_status = CLOSED; + enum { OPEN, CLOSING } m_status = OPEN; llhttp_settings_t m_httpParserSettings; llhttp_t m_httpParser; Intrusive::List::iterator m_currentExecutor; @@ -1217,22 +1161,15 @@ struct PCSX::WebClient::WebClientImpl { multipart_parser* m_multipartParser; multipart_parser_settings m_multipartParserCallbacks; - bool m_closeScheduled = false; }; -PCSX::WebClient::WebClient(WebServer* server) : m_impl(std::make_unique(server, this)) {} +PCSX::WebClient::WebClient(WebServer* server, IO connection, uv_loop_t* loop) + : m_impl(std::make_unique(server, this, connection, loop)) {} void PCSX::WebClient::close() { m_impl->close(); } -bool PCSX::WebClient::accept(uv_tcp_t* srv) { return m_impl->accept(srv); } void PCSX::WebClient::write(Slice&& slice) { m_impl->write(std::move(slice)); } void PCSX::WebClient::write(std::string&& str) { m_impl->write(std::move(str)); } void PCSX::WebClient::write(const std::string& str) { m_impl->write(str); } -void PCSX::WebServer::onNewConnection(int status) { - if (status < 0) return; - WebClient* client = new WebClient(this); - if (client->accept(&m_server)) { - m_clients.push_back(client); - } else { - delete client; - } +void PCSX::WebServer::onConnection(IO connection) { + m_clients.push_back(new WebClient(this, connection, g_system->getLoop())); } diff --git a/src/core/web-server.h b/src/core/web-server.h index 9a0c7d0a8..92f752f5d 100644 --- a/src/core/web-server.h +++ b/src/core/web-server.h @@ -30,6 +30,7 @@ #include "json.hpp" #include "support/eventbus.h" #include "support/list.h" +#include "support/network.h" #include "support/slice.h" namespace PCSX { @@ -100,10 +101,9 @@ class WebExecutor : public Intrusive::List::Node { class WebClient : public Intrusive::List::Node { public: - WebClient(WebServer* server); + WebClient(WebServer* server, IO connection, uv_loop_t* loop); typedef Intrusive::List ListType; void close(); - bool accept(uv_tcp_t* srv); void write(Slice&& slice); template void write(const char (&str)[L]) { @@ -121,32 +121,20 @@ class WebClient : public Intrusive::List::Node { friend WebServer; }; -class WebServer { +class WebServer : public Network::Server { public: WebServer(); ~WebServer() { m_executors.destroyAll(); } - enum WebServerStatus { - SERVER_STOPPED, - SERVER_STOPPING, - SERVER_STARTED, - }; - WebServerStatus getServerStatus() { return m_serverStatus; } - void startServer(uv_loop_t* loop, int port = 8080); - void stopServer(); + protected: + void onConnection(IO connection) override; + void onStopped() override; private: - void onNewConnection(int status); - static void closeCB(uv_handle_t* handle); - WebServerStatus m_serverStatus = SERVER_STOPPED; - uv_tcp_t m_server; - uv_loop_t* m_loop; WebClient::ListType m_clients; EventBus::Listener m_listener; Intrusive::List m_executors; - std::string m_gotError; - friend struct WebClient::WebClientImpl; }; diff --git a/src/gui/gui.cc b/src/gui/gui.cc index 29187f3ce..bd6a672c7 100644 --- a/src/gui/gui.cc +++ b/src/gui/gui.cc @@ -2270,10 +2270,10 @@ the gdb server system itself.)")); if (ImGui::Checkbox(_("Enable Web Server"), &debugSettings.get().value)) { changed = true; if (debugSettings.get()) { - g_emulator->m_webServer->startServer(g_system->getLoop(), - debugSettings.get()); + g_emulator->m_webServer->start(g_system->getLoop(), + debugSettings.get()); } else { - g_emulator->m_webServer->stopServer(); + g_emulator->m_webServer->stop(); } } ImGuiHelpers::ShowHelpMarker(_(R"(This will activate a web-server, that you can From acb1a02c002e5de5f61b0bc104ec4343741903ed Mon Sep 17 00:00:00 2001 From: Nicolas 'Pixel' Noble Date: Tue, 28 Jul 2026 16:49:37 -0700 Subject: [PATCH 09/13] SIO1: move both halves onto the common network endpoint. The server and the client each carried their own three-state enum, and the server set SERVER_STARTED before handing the bind to the listener on the other thread, so its status could never report a failure. Both now derive their state from the transport, which has one. The client also stops throwing std::runtime_error out of an ImGui checkbox callback when raw mode is selected: an unsupported configuration is not an exceptional condition, so it says so and leaves the endpoint alone. Adds onStarting and onStarted hooks to the endpoint bases, since both halves flip an emulator poll flag and hand the connection to SIO1 rather than keeping it themselves. Signed-off-by: Nicolas 'Pixel' Noble --- src/core/sio1-server.cc | 128 ++++++++++++++++------------------------ src/core/sio1-server.h | 39 +++++------- src/gui/gui.cc | 11 ++-- src/support/network.cc | 5 ++ src/support/network.h | 10 ++++ 5 files changed, 85 insertions(+), 108 deletions(-) diff --git a/src/core/sio1-server.cc b/src/core/sio1-server.cc index 27daaf70a..61ee1984e 100644 --- a/src/core/sio1-server.cc +++ b/src/core/sio1-server.cc @@ -22,106 +22,78 @@ #include "core/psxemulator.h" #include "core/sio1.h" -PCSX::SIO1Server::SIO1Server() : m_listener(g_system->m_eventBus) { - m_listener.listen([this](const auto& event) { - if (g_emulator->settings.get().get() && - (m_serverStatus != SIO1ServerStatus::SERVER_STARTED)) { - startServer(g_system->getLoop(), g_emulator->settings.get() - .get()); +namespace { + +// Both halves pick the wire format out of the same setting. +PCSX::SIO1::SIO1Mode currentSIO1Mode() { + auto &debugSettings = PCSX::g_emulator->settings.get(); + auto setting = debugSettings.get().value; + return setting == PCSX::Emulator::DebugSettings::SIO1Mode::Raw ? PCSX::SIO1::SIO1Mode::Raw + : PCSX::SIO1::SIO1Mode::Protobuf; +} + +} // namespace + +PCSX::SIO1Server::SIO1Server() : Network::Server("SIO1 Server"), m_listener(g_system->m_eventBus) { + m_listener.listen([this](const auto &event) { + auto &debugSettings = g_emulator->settings.get(); + if (debugSettings.get() && !isRunning()) { + start(g_system->getLoop(), debugSettings.get()); } }); - m_listener.listen([this](const auto& event) { - if (m_serverStatus == SIO1ServerStatus::SERVER_STARTED) stopServer(); + m_listener.listen([this](const auto &event) { + if (isRunning()) stop(); }); } -void PCSX::SIO1Server::startServer(uv_loop_t* loop, int port) { - if (m_serverStatus == SIO1ServerStatus::SERVER_STARTED) { - throw std::runtime_error("Server already started"); - } - auto& emuSettings = PCSX::g_emulator->settings; - auto& debugSettings = emuSettings.get(); - auto SIO1ModeSettings = debugSettings.get().value; - if (SIO1ModeSettings == Emulator::DebugSettings::SIO1Mode::Raw) { - g_emulator->m_sio1->m_sio1Mode = SIO1::SIO1Mode::Raw; - } else { - g_emulator->m_sio1->m_sio1Mode = SIO1::SIO1Mode::Protobuf; - } +void PCSX::SIO1Server::onStarting() { + g_emulator->m_sio1->m_sio1Mode = currentSIO1Mode(); g_emulator->m_counters->m_pollSIO1 = true; - - m_serverStatus = SIO1ServerStatus::SERVER_STARTED; - m_fifoListener.start(port, loop, &m_async, [this](auto fifo) { - if (fifo) { - g_emulator->m_sio1->setFifo(fifo); - } else { - m_async.data = this; - uv_close(reinterpret_cast(&m_async), [](uv_handle_t* handle) { - SIO1Server* server = reinterpret_cast(handle->data); - server->m_serverStatus = SIO1ServerStatus::SERVER_STOPPED; - }); - } - }); } -void PCSX::SIO1Server::stopServer() { - m_serverStatus = SIO1ServerStatus::SERVER_STOPPING; +void PCSX::SIO1Server::onConnection(IO connection) { g_emulator->m_sio1->setFifo(connection); } + +void PCSX::SIO1Server::onStopped() { g_emulator->m_counters->m_pollSIO1 = false; - m_fifoListener.stop(); g_emulator->m_sio1->stopSIO1Connection(); } -PCSX::SIO1Client::SIO1Client() : m_listener(g_system->m_eventBus) { - m_listener.listen([this](const auto& event) { - if (g_emulator->settings.get().get() && - (m_clientStatus != SIO1ClientStatus::CLIENT_STARTED)) { - startClient(std::string_view(g_emulator->settings.get() - .get() - .value), - g_emulator->settings.get() - .get()); +PCSX::SIO1Client::SIO1Client() : Network::Client("SIO1 Client"), m_listener(g_system->m_eventBus) { + m_listener.listen([this](const auto &event) { + auto &debugSettings = g_emulator->settings.get(); + if (debugSettings.get() && (status() == Network::Status::Stopped)) { + start(g_system->getLoop(), + std::string_view(debugSettings.get().value), + debugSettings.get()); } }); - m_listener.listen([this](const auto& event) { - if (m_clientStatus == SIO1ClientStatus::CLIENT_STARTED) stopClient(); + m_listener.listen([this](const auto &event) { + if (status() != Network::Status::Stopped) stop(); }); } -void PCSX::SIO1Client::startClient(std::string_view address, unsigned port) { - if (m_clientStatus == SIO1ClientStatus::CLIENT_STARTED) { - throw std::runtime_error("Client already started"); - } - - auto& emuSettings = PCSX::g_emulator->settings; - auto& debugSettings = emuSettings.get(); - auto SIO1ModeSettings = debugSettings.get().value; - if (SIO1ModeSettings == Emulator::DebugSettings::SIO1Mode::Raw) { - g_emulator->m_sio1->m_sio1Mode = SIO1::SIO1Mode::Raw; - throw std::runtime_error("Client doesn't currently support raw mode"); - } else { - g_emulator->m_sio1->m_sio1Mode = SIO1::SIO1Mode::Protobuf; - g_emulator->m_counters->m_pollSIO1 = true; - } - - m_clientStatus = SIO1ClientStatus::CLIENT_STARTED; - g_emulator->m_sio1->setFifo(new UvFifo(address, port)); - - if (g_emulator->m_sio1->fifoError()) { - m_clientStatus = SIO1ClientStatus::CLIENT_STOPPING; - g_emulator->m_counters->m_pollSIO1 = false; - stopClient(); +void PCSX::SIO1Client::onStarting() { + auto mode = currentSIO1Mode(); + g_emulator->m_sio1->m_sio1Mode = mode; + if (mode == SIO1::SIO1Mode::Raw) { + // This used to throw std::runtime_error straight out of an ImGui + // checkbox callback. It is a configuration the client does not support, + // not an exceptional condition, so say so and leave the endpoint alone. + g_system->printf("%s", _("SIO1 client does not support raw mode\n")); + return; } + g_emulator->m_counters->m_pollSIO1 = true; } -void PCSX::SIO1Client::reconnect(std::string_view address, unsigned port) { - if (m_clientStatus == SIO1ClientStatus::CLIENT_STARTED) { - m_clientStatus = SIO1ClientStatus::CLIENT_STOPPED; - startClient(address, port); - } -} +void PCSX::SIO1Client::onStarted(IO connection) { g_emulator->m_sio1->setFifo(connection); } -void PCSX::SIO1Client::stopClient() { - m_clientStatus = SIO1ClientStatus::CLIENT_STOPPED; +void PCSX::SIO1Client::onStopped() { g_emulator->m_counters->m_pollSIO1 = false; g_emulator->m_sio1->stopSIO1Connection(); g_system->printf("%s", _("SIO1 client disconnected\n")); } + +void PCSX::SIO1Client::reconnect(std::string_view address, unsigned port) { + stop(); + start(g_system->getLoop(), address, port); +} diff --git a/src/core/sio1-server.h b/src/core/sio1-server.h index f053ac986..3fa9a089f 100644 --- a/src/core/sio1-server.h +++ b/src/core/sio1-server.h @@ -23,46 +23,35 @@ #include #include "support/eventbus.h" -#include "support/uvfile.h" +#include "support/network.h" namespace PCSX { -class SIO1Server { - public: - enum class SIO1ServerStatus { - SERVER_STOPPED, - SERVER_STOPPING, - SERVER_STARTED, - }; - - SIO1ServerStatus getServerStatus() { return m_serverStatus; } +class SIO1Server : public Network::Server { + public: SIO1Server(); - void startServer(uv_loop_t* loop, int port = 6699); - void stopServer(); + + protected: + void onStarting() override; + void onConnection(IO connection) override; + void onStopped() override; private: EventBus::Listener m_listener; - uv_async_t m_async; - SIO1ServerStatus m_serverStatus = SIO1ServerStatus::SERVER_STOPPED; - UvFifoListener m_fifoListener; }; -class SIO1Client { +class SIO1Client : public Network::Client { public: - enum class SIO1ClientStatus { - CLIENT_STOPPED, - CLIENT_STOPPING, - CLIENT_STARTED, - }; - SIO1ClientStatus getClientStatus() { return m_clientStatus; } SIO1Client(); - void startClient(std::string_view address, unsigned port); - void stopClient(); void reconnect(std::string_view address, unsigned port); + protected: + void onStarting() override; + void onStarted(IO connection) override; + void onStopped() override; + private: EventBus::Listener m_listener; - SIO1ClientStatus m_clientStatus = SIO1ClientStatus::CLIENT_STOPPED; }; } // namespace PCSX diff --git a/src/gui/gui.cc b/src/gui/gui.cc index bd6a672c7..2fe187052 100644 --- a/src/gui/gui.cc +++ b/src/gui/gui.cc @@ -2284,10 +2284,10 @@ The debugger might be required in some cases.)")); if (ImGui::Checkbox(_("Enable SIO1 Server"), &debugSettings.get().value)) { changed = true; if (debugSettings.get()) { - g_emulator->m_sio1Server->startServer(g_system->getLoop(), - debugSettings.get()); + g_emulator->m_sio1Server->start(g_system->getLoop(), + debugSettings.get()); } else { - g_emulator->m_sio1Server->stopServer(); + g_emulator->m_sio1Server->stop(); } } ImGuiHelpers::ShowHelpMarker(_(R"(This will activate a tcp server, that will @@ -2298,14 +2298,15 @@ See the wiki for details.)")); if (ImGui::Checkbox(_("Enable SIO1 Client"), &debugSettings.get().value)) { changed = true; if (debugSettings.get()) { - g_emulator->m_sio1Client->startClient( + g_emulator->m_sio1Client->start( + g_system->getLoop(), std::string_view(g_emulator->settings.get() .get() .value), g_emulator->settings.get() .get()); } else { - g_emulator->m_sio1Client->stopClient(); + g_emulator->m_sio1Client->stop(); } } ImGuiHelpers::ShowHelpMarker(_(R"(This will activate a tcp client, that can connect diff --git a/src/support/network.cc b/src/support/network.cc index faf6ca63a..d30a08925 100644 --- a/src/support/network.cc +++ b/src/support/network.cc @@ -73,6 +73,7 @@ void Server::start(uv_loop_t* loop, int port) { if (status() == Status::Running) return; m_loop = loop; m_port = port; + onStarting(); m_async = new uv_async_t(); m_listener.start(port, loop, m_async, [this](UvFifo* fifo) { onListenerEvent(fifo); }); } @@ -132,14 +133,18 @@ void Client::start(uv_loop_t* loop, std::string_view host, int port) { m_loop = loop; m_host = host; m_port = port; + onStarting(); auto fifo = new UvFifo(host, port); m_fifo = fifo; m_connection = IO(fifo); + onStarted(m_connection); } void Client::stop() { + bool wasStarted = m_fifo != nullptr; m_connection.reset(); m_fifo = nullptr; + if (wasStarted) onStopped(); settled(); } diff --git a/src/support/network.h b/src/support/network.h index 2368ba0a0..ace32814d 100644 --- a/src/support/network.h +++ b/src/support/network.h @@ -101,6 +101,10 @@ class Server : public Endpoint { const char* lastError() const override { return m_listener.lastError(); } protected: + // Called before the listener is armed, for whatever the endpoint needs to + // set up first - SIO1 and the ATCons bridge both flip an emulator poll flag + // here rather than reaching into start(). + virtual void onStarting() {} // A client connected. The endpoint takes ownership. virtual void onConnection(IO connection) = 0; // The listener is finished - either an orderly stop, or a bind/listen @@ -146,6 +150,12 @@ class Client : public Endpoint { IO connection() { return m_connection; } protected: + virtual void onStarting() {} + // The outgoing connection was created. It may still be connecting; check + // status() rather than assuming this means connected. + virtual void onStarted(IO connection) {} + virtual void onStopped() {} + void restartNow(uv_loop_t* loop, int port) override { start(loop, m_host, port); } IO m_connection; From d5e9102f3dc5f4bf4c0699042c99692ea4b2039a Mon Sep 17 00:00:00 2001 From: Nicolas 'Pixel' Noble Date: Tue, 28 Jul 2026 17:02:32 -0700 Subject: [PATCH 10/13] Add a Network window. The network services were a flat run of checkboxes buried in the emulation configuration window, with nothing anywhere indicating whether a server was listening or a client connected. A failed bind left the checkbox ticked and said nothing at all. Move them into their own window, one section per service, each with a status indicator, the state in words, a restart button and the error string when there is one. The indicator is drawn rather than glyphed so it does not depend on the font. Also drops ImGuiInputTextFlags_CharsDecimal from the SIO1 client host field, which made it impossible to type a hostname into it. Signed-off-by: Nicolas 'Pixel' Noble --- src/gui/gui.cc | 139 +--------------------- src/gui/gui.h | 5 +- src/gui/widgets/network.cc | 232 +++++++++++++++++++++++++++++++++++++ src/gui/widgets/network.h | 52 +++++++++ 4 files changed, 293 insertions(+), 135 deletions(-) create mode 100644 src/gui/widgets/network.cc create mode 100644 src/gui/widgets/network.h diff --git a/src/gui/gui.cc b/src/gui/gui.cc index 2fe187052..8283dd198 100644 --- a/src/gui/gui.cc +++ b/src/gui/gui.cc @@ -1451,6 +1451,7 @@ in Configuration->Emulation, restart PCSX-Redux, then try again.)")); ImGui::MenuItem(_("Show SPU debug"), nullptr, &PCSX::g_emulator->m_spu->m_showDebug); ImGui::EndMenu(); } + ImGui::MenuItem(_("Show Network"), nullptr, &m_network.m_show); if (ImGui::BeginMenu(_("CD-Rom"))) { ImGui::MenuItem(_("Show Iso Browser"), nullptr, &m_isoBrowser.m_show); ImGui::MenuItem(_("Show CD-ROM viewer"), nullptr, &m_cdromViewer.m_show); @@ -1769,6 +1770,8 @@ in Configuration->Emulation, restart PCSX-Redux, then try again.)")); m_isoBrowser.draw(g_emulator->m_cdrom.get(), _("ISO Browser")); } + if (m_network.m_show) changed |= m_network.draw(this, _("Network")); + if (m_showCfg) changed |= configure(); if (g_emulator->m_spu->m_showCfg) changed |= g_emulator->m_spu->configure(); g_emulator->m_spu->debug(); @@ -2224,140 +2227,8 @@ faster by not displaying the logo.)")); ImGuiHelpers::ShowHelpMarker(_(R"(This will enable the usage of various breakpoints throughout the execution of mips code. Enabling this can slow down emulation to a noticeable extent.)")); - if (ImGui::Checkbox(_("Enable GDB Server"), &debugSettings.get().value)) { - changed = true; - if (debugSettings.get()) { - g_emulator->m_gdbServer->start(g_system->getLoop(), - debugSettings.get()); - } else { - g_emulator->m_gdbServer->stop(); - } - } - ImGuiHelpers::ShowHelpMarker(_(R"(This will activate a gdb-server that you can -connect to with any gdb-remote compliant client. -You also need to enable the debugger.)")); - changed |= - ImGui::Checkbox(_("GDB send manifest"), &debugSettings.get().value); - ImGuiHelpers::ShowHelpMarker(_(R"(Enables sending the processor's manifest -from the gdb server. Keep this enabled, unless -you want to connect IDA to this server, as it -has a bug in its manifest parser.)")); - auto& currentGdbLog = debugSettings.get().value; - auto currentName = magic_enum::enum_name(currentGdbLog); - - if (ImGui::BeginCombo(_("PCSX Logs to GDB"), currentName.data())) { - for (auto v : magic_enum::enum_values()) { - bool selected = (v == currentGdbLog); - auto name = magic_enum::enum_name(v); - if (ImGui::Selectable(name.data(), selected)) { - currentGdbLog = v; - changed = true; - } - if (selected) { - ImGui::SetItemDefaultFocus(); - } - } - ImGui::EndCombo(); - } - - changed |= - ImGui::InputInt(_("GDB Server Port"), &debugSettings.get().value); - changed |= - ImGui::Checkbox(_("GDB Server Trace"), &debugSettings.get().value); - ImGuiHelpers::ShowHelpMarker(_(R"(The GDB server will start tracing its -protocol into the logs, which can be helpful to debug -the gdb server system itself.)")); - if (ImGui::Checkbox(_("Enable Web Server"), &debugSettings.get().value)) { - changed = true; - if (debugSettings.get()) { - g_emulator->m_webServer->start(g_system->getLoop(), - debugSettings.get()); - } else { - g_emulator->m_webServer->stop(); - } - } - ImGuiHelpers::ShowHelpMarker(_(R"(This will activate a web-server, that you can -query using a REST api. See the wiki for details. -The debugger might be required in some cases.)")); - changed |= - ImGui::InputInt(_("Web Server Port"), &debugSettings.get().value); - if (ImGui::Checkbox(_("Enable SIO1 Server"), &debugSettings.get().value)) { - changed = true; - if (debugSettings.get()) { - g_emulator->m_sio1Server->start(g_system->getLoop(), - debugSettings.get()); - } else { - g_emulator->m_sio1Server->stop(); - } - } - ImGuiHelpers::ShowHelpMarker(_(R"(This will activate a tcp server, that will -relay information between tcp and sio1. -See the wiki for details.)")); - changed |= - ImGui::InputInt(_("SIO1 Server Port"), &debugSettings.get().value); - if (ImGui::Checkbox(_("Enable SIO1 Client"), &debugSettings.get().value)) { - changed = true; - if (debugSettings.get()) { - g_emulator->m_sio1Client->start( - g_system->getLoop(), - std::string_view(g_emulator->settings.get() - .get() - .value), - g_emulator->settings.get() - .get()); - } else { - g_emulator->m_sio1Client->stop(); - } - } - ImGuiHelpers::ShowHelpMarker(_(R"(This will activate a tcp client, that can connect -to another PCSX-Redux server to relay information between tcp and sio1. -See the wiki for details.)")); - changed |= - ImGui::InputText(_("SIO1 Client Host"), &debugSettings.get().value, - ImGuiInputTextFlags_CharsDecimal); - changed |= - ImGui::InputInt(_("SIO1 Client Port"), &debugSettings.get().value); - - auto& currentSIO1Mode = debugSettings.get().value; - auto currentSIO1Name = magic_enum::enum_name(currentSIO1Mode); - if (ImGui::Button(_("Reset SIO"))) { - g_emulator->m_sio1->reset(); - } - - const bool enableReconnect = debugSettings.get() && - !g_emulator->m_sio1->connecting() && g_emulator->m_sio1->fifoError(); - - if (!enableReconnect) { - ImGui::BeginDisabled(); - } - - if (ImGui::Button(_("Reconnect"))) { - g_emulator->m_sio1Client->reconnect( - std::string_view(g_emulator->settings.get() - .get() - .value), - g_emulator->settings.get() - .get()); - } - - if (!enableReconnect) { - ImGui::EndDisabled(); - } - - if (ImGui::BeginCombo(_("SIO1Mode"), currentSIO1Name.data())) { - for (auto v : magic_enum::enum_values()) { - bool selected = (v == currentSIO1Mode); - auto name = magic_enum::enum_name(v); - if (ImGui::Selectable(name.data(), selected)) { - currentSIO1Mode = v; - changed = true; - } - if (selected) { - ImGui::SetItemDefaultFocus(); - } - } - ImGui::EndCombo(); - } + ImGui::TextUnformatted(_("Network servers and clients now live in their own window.")); + if (ImGui::Button(_("Open Network settings"))) m_network.m_show = true; } ImGui::End(); diff --git a/src/gui/gui.h b/src/gui/gui.h index ba03d36b5..25473dacc 100644 --- a/src/gui/gui.h +++ b/src/gui/gui.h @@ -49,6 +49,7 @@ #include "gui/widgets/heap_viewer.h" #include "gui/widgets/hwregs.h" #include "gui/widgets/isobrowser.h" +#include "gui/widgets/network.h" #include "gui/widgets/kernellog.h" #include "gui/widgets/log.h" #include "gui/widgets/luaeditor.h" @@ -116,6 +117,7 @@ class GUI final : public UI { typedef Setting ShowKernelLog; typedef Setting ShowCallstacks; typedef Setting ShowSIO1; + typedef Setting ShowNetwork; typedef Setting ShowIsoBrowser; typedef Setting ShowGPULogger; typedef Setting ShowRAMViewer; @@ -167,7 +169,7 @@ class GUI final : public UI { ShowCLUTVRAMViewer, ShowVRAMViewer1, ShowVRAMViewer2, ShowVRAMViewer3, ShowVRAMViewer4, ShowMemoryObserver, ShowTypedDebugger, ShowPatches, ShowMemcardManager, ShowRegisters, ShowAssembly, ShowDisassembly, ShowBreakpoints, ShowNamedSaveStates, ShowEvents, ShowHandlers, ShowKernelLog, ShowCallstacks, ShowSIO1, - ShowIsoBrowser, ShowGPULogger, ShowRAMViewer, ShowCDRomViewer, ShowHeapViewer, ShowHWRegs, MainFontSize, + ShowNetwork, ShowIsoBrowser, ShowGPULogger, ShowRAMViewer, ShowCDRomViewer, ShowHeapViewer, ShowHWRegs, MainFontSize, MonoFontSize, GUITheme, AllowMouseCaptureToggle, EnableRawMouseMotion, WidescreenRatio, ShowPIOCartConfig, ShowMemoryEditor1, ShowMemoryEditor2, ShowMemoryEditor3, ShowMemoryEditor4, ShowMemoryEditor5, ShowMemoryEditor6, ShowMemoryEditor7, ShowMemoryEditor8, ShowParallelPortEditor, ShowScratchpadEditor, @@ -410,6 +412,7 @@ class GUI final : public UI { Widgets::NamedSaveStates m_namedSaveStates = {settings.get().value}; Widgets::Breakpoints m_breakpoints = {settings.get().value}; Widgets::IsoBrowser m_isoBrowser; + Widgets::Network m_network = {settings.get().value}; bool m_showCfg = false; bool m_showUiCfg = false; diff --git a/src/gui/widgets/network.cc b/src/gui/widgets/network.cc new file mode 100644 index 000000000..b4ed578e9 --- /dev/null +++ b/src/gui/widgets/network.cc @@ -0,0 +1,232 @@ +/*************************************************************************** + * Copyright (C) 2026 PCSX-Redux authors * + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + * This program is distributed in the hope that it will be useful, * + * but WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * + * GNU General Public License for more details. * + * * + * You should have received a copy of the GNU General Public License * + * along with this program; if not, write to the * + * Free Software Foundation, Inc., * + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. * + ***************************************************************************/ + +#include "gui/widgets/network.h" + +#include + +#include "core/gdb-server.h" +#include "core/psxemulator.h" +#include "core/sio1-server.h" +#include "core/sio1.h" +#include "core/system.h" +#include "core/web-server.h" +#include "gui/gui.h" +#include "imgui.h" +#include "imgui_stdlib.h" +#include "support/imgui-helpers.h" + +namespace { + +ImU32 statusColor(PCSX::Network::Status status) { + switch (status) { + case PCSX::Network::Status::Stopped: + return IM_COL32(128, 128, 128, 255); + case PCSX::Network::Status::Starting: + return IM_COL32(230, 180, 40, 255); + case PCSX::Network::Status::Running: + return IM_COL32(60, 200, 60, 255); + case PCSX::Network::Status::Failed: + return IM_COL32(220, 60, 60, 255); + } + return IM_COL32(128, 128, 128, 255); +} + +} // namespace + +void PCSX::Widgets::Network::drawStatus(const PCSX::Network::Endpoint* endpoint) { + // Drawn rather than glyphed, so this needs nothing from the font. + const float radius = ImGui::GetTextLineHeight() * 0.28f; + const ImVec2 cursor = ImGui::GetCursorScreenPos(); + const ImVec2 center(cursor.x + radius + 2.0f, cursor.y + ImGui::GetTextLineHeight() * 0.5f); + ImGui::GetWindowDrawList()->AddCircleFilled(center, radius, statusColor(endpoint->status())); + ImGui::Dummy(ImVec2((radius + 2.0f) * 2.0f, ImGui::GetTextLineHeight())); + ImGui::SameLine(); +} + +bool PCSX::Widgets::Network::drawEndpointHeader(PCSX::Network::Endpoint* endpoint, bool enabled) { + bool restarted = false; + drawStatus(endpoint); + ImGui::Text("%s", endpoint->name().data()); + ImGui::SameLine(); + ImGui::TextDisabled("(%s)", PCSX::Network::toString(endpoint->status())); + + // Restarting something that was never switched on is meaningless, and so is + // restarting while a bind or connect is still in flight. + const auto status = endpoint->status(); + const bool canRestart = + enabled && ((status == PCSX::Network::Status::Running) || (status == PCSX::Network::Status::Failed)); + ImGui::SameLine(); + if (!canRestart) ImGui::BeginDisabled(); + ImGui::PushID(endpoint); + if (ImGui::SmallButton(_("Restart"))) { + endpoint->restart(); + restarted = true; + } + ImGui::PopID(); + if (!canRestart) ImGui::EndDisabled(); + + // The whole point of the exercise: a failed bind used to be completely + // silent, with the checkbox still ticked. + const char* error = endpoint->lastError(); + if ((status == PCSX::Network::Status::Failed) && error && error[0]) { + ImGui::PushStyleColor(ImGuiCol_Text, ImVec4(0.9f, 0.3f, 0.3f, 1.0f)); + ImGui::TextWrapped("%s", error); + ImGui::PopStyleColor(); + } + return restarted; +} + +bool PCSX::Widgets::Network::draw(GUI* gui, const char* title) { + if (!ImGui::Begin(title, &m_show)) { + ImGui::End(); + return false; + } + + bool changed = false; + auto& debugSettings = g_emulator->settings.get(); + + // -- GDB server -- + if (ImGui::CollapsingHeader(_("GDB Server"), ImGuiTreeNodeFlags_DefaultOpen)) { + drawEndpointHeader(g_emulator->m_gdbServer.get(), debugSettings.get()); + if (ImGui::Checkbox(_("Enable GDB Server"), &debugSettings.get().value)) { + changed = true; + if (debugSettings.get()) { + g_emulator->m_gdbServer->start(g_system->getLoop(), + debugSettings.get()); + } else { + g_emulator->m_gdbServer->stop(); + } + } + ImGuiHelpers::ShowHelpMarker(_(R"(This will activate a gdb-server that you can +connect to with any gdb-remote compliant client. +You also need to enable the debugger.)")); + changed |= + ImGui::InputInt(_("GDB Server Port"), &debugSettings.get().value); + ImGuiHelpers::ShowHelpMarker(_(R"(A port change only takes effect on the +next restart of the server.)")); + changed |= + ImGui::Checkbox(_("GDB send manifest"), &debugSettings.get().value); + ImGuiHelpers::ShowHelpMarker(_(R"(Enables sending the processor's manifest +from the gdb server. Keep this enabled, unless +you want to connect IDA to this server, as it +has a bug in its manifest parser.)")); + changed |= + ImGui::Checkbox(_("GDB Server Trace"), &debugSettings.get().value); + ImGuiHelpers::ShowHelpMarker(_(R"(The GDB server will start tracing its +protocol into the logs, which can be helpful to debug +the gdb server system itself.)")); + auto& currentGdbLog = debugSettings.get().value; + auto currentGdbLogName = magic_enum::enum_name(currentGdbLog); + if (ImGui::BeginCombo(_("PCSX Logs to GDB"), currentGdbLogName.data())) { + for (auto v : magic_enum::enum_values()) { + bool selected = (v == currentGdbLog); + auto name = magic_enum::enum_name(v); + if (ImGui::Selectable(name.data(), selected)) { + currentGdbLog = v; + changed = true; + } + if (selected) ImGui::SetItemDefaultFocus(); + } + ImGui::EndCombo(); + } + } + + // -- Web server -- + if (ImGui::CollapsingHeader(_("Web Server"), ImGuiTreeNodeFlags_DefaultOpen)) { + drawEndpointHeader(g_emulator->m_webServer.get(), debugSettings.get()); + if (ImGui::Checkbox(_("Enable Web Server"), &debugSettings.get().value)) { + changed = true; + if (debugSettings.get()) { + g_emulator->m_webServer->start(g_system->getLoop(), + debugSettings.get()); + } else { + g_emulator->m_webServer->stop(); + } + } + ImGuiHelpers::ShowHelpMarker(_(R"(This will activate a web-server, that you can +query using a REST api. See the wiki for details.)")); + changed |= + ImGui::InputInt(_("Web Server Port"), &debugSettings.get().value); + ImGuiHelpers::ShowHelpMarker(_(R"(A port change only takes effect on the +next restart of the server.)")); + } + + // -- SIO1 -- + if (ImGui::CollapsingHeader(_("SIO1"), ImGuiTreeNodeFlags_DefaultOpen)) { + drawEndpointHeader(g_emulator->m_sio1Server.get(), debugSettings.get()); + if (ImGui::Checkbox(_("Enable SIO1 Server"), &debugSettings.get().value)) { + changed = true; + if (debugSettings.get()) { + g_emulator->m_sio1Server->start(g_system->getLoop(), + debugSettings.get()); + } else { + g_emulator->m_sio1Server->stop(); + } + } + ImGuiHelpers::ShowHelpMarker(_(R"(This will activate a tcp server, that will +relay information between tcp and sio1. +See the wiki for details.)")); + changed |= ImGui::InputInt(_("SIO1 Server Port"), + &debugSettings.get().value); + + ImGui::Separator(); + + drawEndpointHeader(g_emulator->m_sio1Client.get(), debugSettings.get()); + if (ImGui::Checkbox(_("Enable SIO1 Client"), &debugSettings.get().value)) { + changed = true; + if (debugSettings.get()) { + g_emulator->m_sio1Client->start( + g_system->getLoop(), + std::string_view(debugSettings.get().value), + debugSettings.get()); + } else { + g_emulator->m_sio1Client->stop(); + } + } + // This used to be flagged CharsDecimal, so a hostname could not be + // typed into the hostname field. + changed |= ImGui::InputText(_("SIO1 Client Host"), + &debugSettings.get().value); + changed |= ImGui::InputInt(_("SIO1 Client Port"), + &debugSettings.get().value); + + if (ImGui::Button(_("Reset SIO"))) { + g_emulator->m_sio1->reset(); + } + + auto& currentSIO1Mode = debugSettings.get().value; + auto currentSIO1Name = magic_enum::enum_name(currentSIO1Mode); + if (ImGui::BeginCombo(_("SIO1Mode"), currentSIO1Name.data())) { + for (auto v : magic_enum::enum_values()) { + bool selected = (v == currentSIO1Mode); + auto name = magic_enum::enum_name(v); + if (ImGui::Selectable(name.data(), selected)) { + currentSIO1Mode = v; + changed = true; + } + if (selected) ImGui::SetItemDefaultFocus(); + } + ImGui::EndCombo(); + } + } + + ImGui::End(); + return changed; +} diff --git a/src/gui/widgets/network.h b/src/gui/widgets/network.h new file mode 100644 index 000000000..137349e8b --- /dev/null +++ b/src/gui/widgets/network.h @@ -0,0 +1,52 @@ +/*************************************************************************** + * Copyright (C) 2026 PCSX-Redux authors * + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + * This program is distributed in the hope that it will be useful, * + * but WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * + * GNU General Public License for more details. * + * * + * You should have received a copy of the GNU General Public License * + * along with this program; if not, write to the * + * Free Software Foundation, Inc., * + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. * + ***************************************************************************/ + +#pragma once + +#include "support/network.h" + +namespace PCSX { + +class GUI; + +namespace Widgets { + +// The network services used to be a flat run of checkboxes buried in the +// emulation configuration window, with no indication anywhere of whether a +// server was actually listening or a client actually connected. A failed bind +// left the checkbox ticked and said nothing at all. +class Network { + public: + Network(bool& show) : m_show(show) {} + bool draw(GUI* gui, const char* title); + + bool& m_show; + + private: + // Grey stopped, amber connecting, green up, red failed. Returns the width + // consumed so the rows line up. + void drawStatus(const PCSX::Network::Endpoint* endpoint); + // Shared row furniture: bullet, name, state, and a restart button that is + // only live when there is something to restart. + bool drawEndpointHeader(PCSX::Network::Endpoint* endpoint, bool enabled); +}; + +} // namespace Widgets + +} // namespace PCSX From 9e52a387df00edb6b0268383851fc18e86ef60e8 Mon Sep 17 00:00:00 2001 From: Nicolas 'Pixel' Noble Date: Tue, 28 Jul 2026 20:23:58 -0700 Subject: [PATCH 11/13] Fix the CI failures on the network series. The Windows build lists its sources explicitly, so the two new files were never compiled there and the link failed on every symbol in them. Add them to the support, gui and test projects, and to the filters so they land in the right folder. asan caught two leak families. uv_connect_t was deleted on the connect failure path and never on success, which the new tests are the first thing to exercise. And the tests themselves ignored uv_loop_close returning EBUSY, which leaks the loop's internals whenever a handle is still open - so close the handles first and pump until the close takes. Signed-off-by: Nicolas 'Pixel' Noble --- src/support/uvfile.cc | 4 ++- tests/support/network.cc | 27 ++++++++++----- tests/support/uvfifolistener.cc | 35 ++++++++++++++++---- vsprojects/gui/gui.vcxproj | 2 ++ vsprojects/gui/gui.vcxproj.filters | 3 ++ vsprojects/support/support.vcxproj | 2 ++ vsprojects/support/support.vcxproj.filters | 3 ++ vsprojects/tests/support/testsupport.vcxproj | 2 ++ 8 files changed, 62 insertions(+), 16 deletions(-) diff --git a/src/support/uvfile.cc b/src/support/uvfile.cc index 32bf87a8c..b1076a7a3 100644 --- a/src/support/uvfile.cc +++ b/src/support/uvfile.cc @@ -815,7 +815,9 @@ PCSX::UvFifo::UvFifo(const std::string_view address, unsigned port) : File(File: return; } fifo->m_connecting.clear(); - fifo->startRead(reinterpret_cast(connect->handle)); + auto handle = reinterpret_cast(connect->handle); + delete connect; + fifo->startRead(handle); }); if (result != 0) { m_connectErrorCode.store(result, std::memory_order_release); diff --git a/tests/support/network.cc b/tests/support/network.cc index 1e61c6e29..9943429b5 100644 --- a/tests/support/network.cc +++ b/tests/support/network.cc @@ -45,6 +45,16 @@ void pump(uv_loop_t* loop, int milliseconds = 300) { } } +// uv_loop_close returns EBUSY while any handle is still open or still closing, +// and ignoring that return leaks the loop's internals. Close everything first, +// then keep pumping until it actually takes. +void closeLoop(uv_loop_t* loop) { + for (int i = 0; (i < 200) && (uv_loop_close(loop) == UV_EBUSY); i++) { + uv_run(loop, UV_RUN_NOWAIT); + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + } +} + // Holds a port so a Server's bind is guaranteed to fail. Releasable mid-test, // so the restart case can watch a Failed endpoint recover. struct Squatter { @@ -122,8 +132,8 @@ TEST(NetworkServer, AcceptsAndReportsRunning) { EXPECT_EQ(server.status(), Network::Status::Stopped); EXPECT_EQ(server.m_stoppedCount, 1); - uv_run(&loop, UV_RUN_NOWAIT); - uv_loop_close(&loop); + pump(&loop); + closeLoop(&loop); } TEST(NetworkServer, BindFailureIsVisible) { @@ -143,8 +153,9 @@ TEST(NetworkServer, BindFailureIsVisible) { // A failure is a teardown, so subclasses get told once and only once. EXPECT_EQ(server.m_stoppedCount, 1); - uv_run(&loop, UV_RUN_NOWAIT); - uv_loop_close(&loop); + squatter.release(&loop); + pump(&loop); + closeLoop(&loop); } // The restart button's actual job: recover an endpoint that is sitting in @@ -176,8 +187,8 @@ TEST(NetworkServer, RestartRecoversFromFailure) { pump(&loop); EXPECT_EQ(server.status(), Network::Status::Stopped); - uv_run(&loop, UV_RUN_NOWAIT); - uv_loop_close(&loop); + pump(&loop); + closeLoop(&loop); } TEST(NetworkClient, ConnectFailureIsVisible) { @@ -200,8 +211,8 @@ TEST(NetworkClient, ConnectFailureIsVisible) { client.stop(); EXPECT_EQ(client.status(), Network::Status::Stopped); - uv_run(&loop, UV_RUN_NOWAIT); - uv_loop_close(&loop); + pump(&loop); + closeLoop(&loop); } // The UI iterates this instead of hard-coding a row per service, so an endpoint diff --git a/tests/support/uvfifolistener.cc b/tests/support/uvfifolistener.cc index b68e58c22..0a41437d6 100644 --- a/tests/support/uvfifolistener.cc +++ b/tests/support/uvfifolistener.cc @@ -48,6 +48,16 @@ void pump(uv_loop_t* loop, int milliseconds = 250) { } } +// uv_loop_close returns EBUSY while any handle is still open or still closing, +// and ignoring that return leaks the loop's internals. Close everything first, +// then keep pumping until it actually takes. +void closeLoop(uv_loop_t* loop) { + for (int i = 0; (i < 200) && (uv_loop_close(loop) == UV_EBUSY); i++) { + uv_run(loop, UV_RUN_NOWAIT); + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + } +} + // Takes a port and holds it, so UvFifoListener's bind is guaranteed to fail. struct Squatter { explicit Squatter(uv_loop_t* loop, unsigned port) { @@ -57,8 +67,16 @@ struct Squatter { EXPECT_EQ(uv_tcp_bind(&m_tcp, reinterpret_cast(&addr), 0), 0); EXPECT_EQ(uv_listen(reinterpret_cast(&m_tcp), 16, [](uv_stream_t*, int) {}), 0); } - ~Squatter() { uv_close(reinterpret_cast(&m_tcp), [](uv_handle_t*) {}); } + void release(uv_loop_t* loop) { + if (m_released) return; + m_released = true; + uv_close(reinterpret_cast(&m_tcp), [](uv_handle_t*) {}); + } + ~Squatter() { + if (!m_released) uv_close(reinterpret_cast(&m_tcp), [](uv_handle_t*) {}); + } uv_tcp_t m_tcp = {}; + bool m_released = false; }; } // namespace @@ -89,8 +107,9 @@ TEST(UvFifoListener, StartStopOnFreePort) { EXPECT_EQ(listener.status(), UvFifoListener::Status::Stopped); EXPECT_EQ(nullptrCallbacks, 1); - uv_run(&loop, UV_RUN_NOWAIT); - uv_loop_close(&loop); + uv_close(reinterpret_cast(&async), [](uv_handle_t*) {}); + pump(&loop); + closeLoop(&loop); } // The bug: start() on an occupied port hits the uv_tcp_bind failure path, which @@ -130,8 +149,10 @@ TEST(UvFifoListener, StopAfterFailedBindIsSafe) { EXPECT_EQ(listener.status(), UvFifoListener::Status::Failed); - uv_run(&loop, UV_RUN_NOWAIT); - uv_loop_close(&loop); + uv_close(reinterpret_cast(&async), [](uv_handle_t*) {}); + squatter.release(&loop); + pump(&loop); + closeLoop(&loop); } // UvFifo has no readable callback of its own - data lands in a lock-free queue @@ -194,7 +215,7 @@ TEST(UvFifo, ReadableNotifierFires) { uv_close(reinterpret_cast(¬ifyAsync), [](uv_handle_t*) {}); uv_close(reinterpret_cast(&listenerAsync), [](uv_handle_t*) {}); pump(&loop); - uv_loop_close(&loop); + closeLoop(&loop); } // A failed outgoing connection has to end up in a state the UI can act on. @@ -310,5 +331,5 @@ TEST(UvFifo, CloseFlushesPendingWrites) { pump(&loop); uv_close(reinterpret_cast(&listenerAsync), [](uv_handle_t*) {}); pump(&loop); - uv_loop_close(&loop); + closeLoop(&loop); } diff --git a/vsprojects/gui/gui.vcxproj b/vsprojects/gui/gui.vcxproj index ed87a3b53..6791c4f0a 100644 --- a/vsprojects/gui/gui.vcxproj +++ b/vsprojects/gui/gui.vcxproj @@ -146,6 +146,7 @@ + @@ -186,6 +187,7 @@ + diff --git a/vsprojects/gui/gui.vcxproj.filters b/vsprojects/gui/gui.vcxproj.filters index f90cdbbfd..c7388b67d 100644 --- a/vsprojects/gui/gui.vcxproj.filters +++ b/vsprojects/gui/gui.vcxproj.filters @@ -91,6 +91,9 @@ Source Files\widgets + + Source Files\widgets + Source Files\widgets diff --git a/vsprojects/support/support.vcxproj b/vsprojects/support/support.vcxproj index bb5007f93..995c7dfdd 100644 --- a/vsprojects/support/support.vcxproj +++ b/vsprojects/support/support.vcxproj @@ -178,6 +178,7 @@ + @@ -221,6 +222,7 @@ + diff --git a/vsprojects/support/support.vcxproj.filters b/vsprojects/support/support.vcxproj.filters index 8e1104d6c..7615d2f87 100644 --- a/vsprojects/support/support.vcxproj.filters +++ b/vsprojects/support/support.vcxproj.filters @@ -188,6 +188,9 @@ Source Files + + Source Files + Source Files diff --git a/vsprojects/tests/support/testsupport.vcxproj b/vsprojects/tests/support/testsupport.vcxproj index 58631ebea..dff68ffab 100644 --- a/vsprojects/tests/support/testsupport.vcxproj +++ b/vsprojects/tests/support/testsupport.vcxproj @@ -86,6 +86,8 @@ + + From d123dd7cdf763a1391c9ad6ec9a371f935821ba9 Mon Sep 17 00:00:00 2001 From: Nicolas 'Pixel' Noble Date: Wed, 29 Jul 2026 06:57:34 -0700 Subject: [PATCH 12/13] Link libuv and libcurl into the support test project. The new network tests are the first thing in that project to use libuv directly, and it only referenced gtest, supportpsx, support, tracy and the leak detector, so the Windows link failed on every uv and curl symbol. pcsxrunner already references both for the same reason. Signed-off-by: Nicolas 'Pixel' Noble --- vsprojects/tests/support/testsupport.vcxproj | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/vsprojects/tests/support/testsupport.vcxproj b/vsprojects/tests/support/testsupport.vcxproj index dff68ffab..ee905b834 100644 --- a/vsprojects/tests/support/testsupport.vcxproj +++ b/vsprojects/tests/support/testsupport.vcxproj @@ -97,6 +97,12 @@ {b2e2ad84-9d7f-4976-9572-e415819ffd7f} + + {25c13988-a8a8-4bfa-962f-0833020e4ee4} + + + {4b88e4f6-56b3-4f66-bee8-0a4a21937bee} + {0e621321-093c-4d60-bd8b-027fdc2b0f63} From 80c8fb5a583cea00cd7fa6c6875981e81ad97d21 Mon Sep 17 00:00:00 2001 From: Nicolas 'Pixel' Noble Date: Wed, 29 Jul 2026 07:46:17 -0700 Subject: [PATCH 13/13] Link the Windows system libraries into the support test project. Pulling libuv in leaves the Winsock symbols undefined, since the project declared no AdditionalDependencies at all and inherited only what common.props provides. Use the same list pcsxrunner already carries. ReleaseWithClangCL had no Link section, which is the configuration CI builds, so it gets one. Signed-off-by: Nicolas 'Pixel' Noble --- vsprojects/tests/support/testsupport.vcxproj | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/vsprojects/tests/support/testsupport.vcxproj b/vsprojects/tests/support/testsupport.vcxproj index ee905b834..3d81372cf 100644 --- a/vsprojects/tests/support/testsupport.vcxproj +++ b/vsprojects/tests/support/testsupport.vcxproj @@ -117,6 +117,9 @@ $(SolutionDir)..\third_party\ucl;$(SolutionDir)..\third_party\ucl\include;%(AdditionalIncludeDirectories) + + imm32.lib;iphlpapi.lib;kernel32.lib;opengl32.lib;psapi.lib;setupapi.lib;shlwapi.lib;userenv.lib;version.lib;winmm.lib;ws2_32.lib;%(AdditionalDependencies) + @@ -134,6 +137,7 @@ true Console + imm32.lib;iphlpapi.lib;kernel32.lib;opengl32.lib;psapi.lib;setupapi.lib;shlwapi.lib;userenv.lib;version.lib;winmm.lib;ws2_32.lib;%(AdditionalDependencies) @@ -149,6 +153,7 @@ true Console + imm32.lib;iphlpapi.lib;kernel32.lib;opengl32.lib;psapi.lib;setupapi.lib;shlwapi.lib;userenv.lib;version.lib;winmm.lib;ws2_32.lib;%(AdditionalDependencies) true true @@ -166,6 +171,7 @@ true Console + imm32.lib;iphlpapi.lib;kernel32.lib;opengl32.lib;psapi.lib;setupapi.lib;shlwapi.lib;userenv.lib;version.lib;winmm.lib;ws2_32.lib;%(AdditionalDependencies) true true