[MG_Util, MG_Remote] (P12 review): the in-process display server forwards only its live session's own log lines to that session's client, and sends them outside the log mutex

SetLogForwarder forwards every server-role thread's line, which is right for a session child (the process is the session) and wrong for the display Activity's process, where MOBILEGL_IPC_ROLE=server makes every thread a server thread: the UI thread's surface callbacks and the listener's lines - naming other peers' addresses, Busy refusals included - went into the live client's control socket. And the forward was a blocking SendFrame under the global log mutex, so a client that stopped reading held that mutex and hung the UI thread at its next log line (surfaceDestroyed's Detach logs).

In-process, RunSession now installs SetSessionLogForwarder: only the session thread and the threads that opt in (the session's apply thread, SetThreadForwardsLogToPeer) forward, and the send happens after the log mutex is released, under a forward mutex of its own (WithLogBarrier takes both, so the flush ack still follows every send that began; a line logged by the send itself is not re-sent). The forked session child keeps SetLogForwarder unchanged; the pull build's Log() is unchanged (G1).
This commit is contained in:
2026-09-24 14:03:47 +00:00
parent 7680b78621
commit ee456ccc40
4 changed files with 73 additions and 3 deletions
+3
View File
@@ -362,6 +362,9 @@ namespace MobileGL::MG_Remote::Server {
// own log. Under spawn the process role already answers this; under INPROC it is the
// only thing that can, because the client role is another thread of this same process.
MG_Util::Debug::SetThreadLogRole(MG_Util::Debug::LogRole::Server);
// P12 review fix: and it is one of the session's own threads, whose lines a session-scoped
// forwarder (the in-process display server's) sends to the client. Inert otherwise.
MG_Util::Debug::SetThreadForwardsLogToPeer(true);
Bool recognised = true;
const char* raw = AffinityStringFromConfig();
+7 -2
View File
@@ -574,8 +574,13 @@ int RunSession(std::unique_ptr<SocketTransport> control, std::vector<std::uint8_
}
const char* forwarding = std::getenv("MOBILEGL_IPC_LOG_FORWARD");
if (tcp && (!forwarding || std::strcmp(forwarding, "0") != 0))
MobileGL::MG_Util::Debug::SetLogForwarder(&ForwardLog, control.get());
if (tcp && (!forwarding || std::strcmp(forwarding, "0") != 0)) {
// P12 review fix: in-process, only THIS session's threads (this one and its apply thread)
// forward, and outside the log mutex - the UI thread, the listener and the next session's
// refusals share this process and must neither reach this client nor wait on it.
if (inProcess) MobileGL::MG_Util::Debug::SetSessionLogForwarder(&ForwardLog, control.get());
else MobileGL::MG_Util::Debug::SetLogForwarder(&ForwardLog, control.get());
}
// p7/spawnhang: this thread posts every surface op the client sends (ServerApplyWireSurfaceOp,
// below) and is the one that says "still running" while the apply thread runs it. Cleared
// after the loop, on this same thread, so no post can be in flight when it goes.
+50 -1
View File
@@ -21,15 +21,42 @@ namespace MobileGL {
static FILE* s_serverLogFile = nullptr;
static LogForwarder s_forwarder = nullptr;
static void* s_forwarderUser = nullptr;
// P12 review fix (SetSessionLogForwarder). Written under BOTH mutexes, so either one is
// enough to read them; the lock order is LogMutex, then ForwardMutex.
static bool s_forwardSessionThreadsOnly = false;
static thread_local bool t_forwardsToPeer = false;
static thread_local bool t_forwarding = false; // a line logged by the send itself is not re-sent
static std::mutex& ForwardMutex() {
static std::mutex mutex;
return mutex;
}
void SetLogForwarder(LogForwarder forwarder, void* user) {
std::lock_guard<std::mutex> lock(LogMutex());
std::lock_guard<std::mutex> forwardLock(ForwardMutex());
s_forwarder = forwarder;
s_forwarderUser = user;
s_forwardSessionThreadsOnly = false;
}
void SetSessionLogForwarder(LogForwarder forwarder, void* user) {
{
std::lock_guard<std::mutex> lock(LogMutex());
std::lock_guard<std::mutex> forwardLock(ForwardMutex());
s_forwarder = forwarder;
s_forwarderUser = user;
s_forwardSessionThreadsOnly = forwarder != nullptr;
}
t_forwardsToPeer = forwarder != nullptr;
}
void SetThreadForwardsLogToPeer(bool forwards) { t_forwardsToPeer = forwards; }
void WithLogBarrier(void (*action)(void*), void* user) {
std::lock_guard<std::mutex> lock(LogMutex());
// A session-scoped line is sent under ForwardMutex after the log mutex was released; the
// barrier's action comes after every such send that has begun.
std::lock_guard<std::mutex> forwardLock(ForwardMutex());
action(user);
}
@@ -244,7 +271,12 @@ namespace MobileGL {
}
void Log(const char* levelTag, android_LogPriority androidLogLevel, const char* fmt, ...) {
#if MOBILEGL_BUILD_DISAGGREGATED
// Unlocked early for a session-scoped forward (SetSessionLogForwarder).
std::unique_lock<std::mutex> lock(LogMutex());
#else
std::lock_guard<std::mutex> lock(LogMutex());
#endif
#if MOBILEGL_LOG_ENABLE_STACKTRACE
auto trace = std::stacktrace::current();
@@ -283,10 +315,27 @@ namespace MobileGL {
WriteToFile(out.c_str());
#if MOBILEGL_BUILD_DISAGGREGATED
if (s_forwarder && ThreadIsServerRole()) s_forwarder(s_forwarderUser, out.c_str());
bool forwardOutsideLock = false;
if (s_forwarder && ThreadIsServerRole() && !t_forwarding) {
if (!s_forwardSessionThreadsOnly) s_forwarder(s_forwarderUser, out.c_str());
else forwardOutsideLock = t_forwardsToPeer;
}
#endif
va_end(args);
#if MOBILEGL_BUILD_DISAGGREGATED
if (forwardOutsideLock) {
// SetSessionLogForwarder: this thread belongs to the live session. The line goes out
// after the log mutex is released, so a stalled client holds only the forward mutex.
lock.unlock();
std::lock_guard<std::mutex> forwardLock(ForwardMutex());
if (s_forwarder != nullptr && s_forwardSessionThreadsOnly) {
t_forwarding = true;
s_forwarder(s_forwarderUser, out.c_str());
t_forwarding = false;
}
}
#endif
}
void Close() {
+13
View File
@@ -138,6 +138,19 @@ namespace MobileGL {
#if MOBILEGL_BUILD_DISAGGREGATED
using LogForwarder = void (*)(void*, const char*);
void SetLogForwarder(LogForwarder forwarder, void* user);
// P12 review fix (log forwarding). THE IN-PROCESS DISPLAY SERVER'S FORWARDER, scoped to
// the live session's own threads. SetLogForwarder forwards every server-role thread's
// line, which is right for a session child (the process IS the session) and wrong for the
// display Activity's process, where the UI thread, the listener (naming other peers'
// addresses) and a previous session share the process: their lines went into the live
// client's socket. This one forwards only from the calling thread and from threads that
// opt in (SetThreadForwardsLogToPeer - the session's apply thread), and it sends OUTSIDE
// the log mutex, under a forward mutex of its own, so a client that stops reading stalls
// the session's threads and not every thread that logs (the UI thread's surface
// callbacks). SetLogForwarder(nullptr, nullptr) clears either kind and waits for a send in
// flight.
void SetSessionLogForwarder(LogForwarder forwarder, void* user);
void SetThreadForwardsLogToPeer(bool forwards);
void WritePeerLog(const char* message);
void WithLogBarrier(void (*action)(void*), void* user);
// P6: ONE LOG PER ROLE, and the role is a per-THREAD fact because under inproc both