From dad4de7faf12e3a09f3395f5dc845b9862e19142 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Tue, 14 Apr 2026 08:21:54 -0700 Subject: [PATCH 1/2] refactor: deduplicate logger mutex and withLock via lockable embedding Extract lockable struct (embedded sync.Mutex + withLock helper) into global_helpers.go and embed it in FileLogger, JSONLLogger, MarkdownLogger, and ToolsLogger. This removes four identical withLock method definitions. Also add CloseAllLoggers() convenience function that closes every global logger in a single call. Fixes #3685 Fixes #3735 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/logger/file_logger.go | 8 +----- internal/logger/global_helpers.go | 43 ++++++++++++++++++++++++++++-- internal/logger/jsonl_logger.go | 8 +----- internal/logger/markdown_logger.go | 8 +----- internal/logger/tools_logger.go | 8 +----- 5 files changed, 45 insertions(+), 30 deletions(-) diff --git a/internal/logger/file_logger.go b/internal/logger/file_logger.go index 10ef3e1f..79a2bd44 100644 --- a/internal/logger/file_logger.go +++ b/internal/logger/file_logger.go @@ -10,9 +10,9 @@ import ( // FileLogger manages logging to a file with fallback to stdout type FileLogger struct { + lockable logFile *os.File logger *log.Logger - mu sync.Mutex logDir string fileName string useFallback bool @@ -56,12 +56,6 @@ func InitFileLogger(logDir, fileName string) error { return err } -// withLock acquires fl.mu, executes fn, then releases fl.mu. -// Use this in methods that return an error to avoid repeating the lock/unlock preamble. -func (fl *FileLogger) withLock(fn func() error) error { - return withMutexLock(&fl.mu, fn) -} - // Close closes the log file func (fl *FileLogger) Close() error { return fl.withLock(func() error { diff --git a/internal/logger/global_helpers.go b/internal/logger/global_helpers.go index 002e0d93..7f75deb3 100644 --- a/internal/logger/global_helpers.go +++ b/internal/logger/global_helpers.go @@ -16,9 +16,29 @@ package logger import "sync" +// lockable provides a mutex and a withLock helper method. Embed this struct +// in logger types that need a sync.Mutex plus a withLock convenience method +// to eliminate the repeated per-type withLock boilerplate. +// +// Usage: +// +// type MyLogger struct { +// lockable +// // other fields... +// } +// +// The embedded mu field and withLock method are promoted, so callers can write +// myLogger.mu.Lock() and myLogger.withLock(fn) directly. +type lockable struct { + mu sync.Mutex +} + +// withLock acquires l.mu, executes fn, then releases l.mu. +func (l *lockable) withLock(fn func() error) error { + return withMutexLock(&l.mu, fn) +} + // withMutexLock acquires mu, calls fn, and releases mu. -// This is the single implementation of the per-type withLock pattern -// used by FileLogger, JSONLLogger, MarkdownLogger, and ToolsLogger. func withMutexLock(mu *sync.Mutex, fn func() error) error { mu.Lock() defer mu.Unlock() @@ -122,3 +142,22 @@ func closeGlobalLogger[T closableLogger](mu *sync.RWMutex, logger *T) error { } return nil } + +// CloseAllLoggers closes all global loggers in a single call. +// Returns the first error encountered, but attempts to close every logger. +func CloseAllLoggers() error { + closers := []func() error{ + CloseGlobalLogger, + CloseJSONLLogger, + CloseMarkdownLogger, + CloseToolsLogger, + CloseServerFileLogger, + } + var firstErr error + for _, fn := range closers { + if err := fn(); err != nil && firstErr == nil { + firstErr = err + } + } + return firstErr +} diff --git a/internal/logger/jsonl_logger.go b/internal/logger/jsonl_logger.go index 0a240bf5..e07ad102 100644 --- a/internal/logger/jsonl_logger.go +++ b/internal/logger/jsonl_logger.go @@ -12,8 +12,8 @@ import ( // JSONLLogger manages logging RPC messages to a JSONL file (one JSON object per line) type JSONLLogger struct { + lockable logFile *os.File - mu sync.Mutex logDir string fileName string encoder *json.Encoder @@ -67,12 +67,6 @@ func InitJSONLLogger(logDir, fileName string) error { return err } -// withLock acquires jl.mu, executes fn, then releases jl.mu. -// Use this in methods that return an error to avoid repeating the lock/unlock preamble. -func (jl *JSONLLogger) withLock(fn func() error) error { - return withMutexLock(&jl.mu, fn) -} - // Close closes the JSONL log file func (jl *JSONLLogger) Close() error { return jl.withLock(func() error { diff --git a/internal/logger/markdown_logger.go b/internal/logger/markdown_logger.go index a695d6c4..4c3653aa 100644 --- a/internal/logger/markdown_logger.go +++ b/internal/logger/markdown_logger.go @@ -11,8 +11,8 @@ import ( // MarkdownLogger manages logging to a markdown file for GitHub workflow previews type MarkdownLogger struct { + lockable logFile *os.File - mu sync.Mutex logDir string fileName string useFallback bool @@ -68,12 +68,6 @@ func (ml *MarkdownLogger) initializeFile() error { return nil } -// withLock acquires ml.mu, executes fn, then releases ml.mu. -// Use this in methods that return an error to avoid repeating the lock/unlock preamble. -func (ml *MarkdownLogger) withLock(fn func() error) error { - return withMutexLock(&ml.mu, fn) -} - // Close closes the log file and writes the closing details tag func (ml *MarkdownLogger) Close() error { return ml.withLock(func() error { diff --git a/internal/logger/tools_logger.go b/internal/logger/tools_logger.go index 074f9ae7..bfdc6ed7 100644 --- a/internal/logger/tools_logger.go +++ b/internal/logger/tools_logger.go @@ -27,10 +27,10 @@ type ToolsData struct { // ToolsLogger manages logging of MCP server tools to a JSON file type ToolsLogger struct { + lockable logDir string fileName string data *ToolsData - mu sync.Mutex useFallback bool } @@ -81,12 +81,6 @@ func InitToolsLogger(logDir, fileName string) error { return err } -// withLock acquires tl.mu, executes fn, then releases tl.mu. -// Use this in methods that return an error to avoid repeating the lock/unlock preamble. -func (tl *ToolsLogger) withLock(fn func() error) error { - return withMutexLock(&tl.mu, fn) -} - // LogTools logs the tools for a specific server func (tl *ToolsLogger) LogTools(serverID string, tools []ToolInfo) error { return tl.withLock(func() error { From d3d08227abf5a744320449bb43a1b7545ce30e79 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Tue, 14 Apr 2026 08:44:15 -0700 Subject: [PATCH 2/2] Update internal/logger/global_helpers.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- internal/logger/global_helpers.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/logger/global_helpers.go b/internal/logger/global_helpers.go index 7f75deb3..0f64e0de 100644 --- a/internal/logger/global_helpers.go +++ b/internal/logger/global_helpers.go @@ -27,8 +27,9 @@ import "sync" // // other fields... // } // -// The embedded mu field and withLock method are promoted, so callers can write -// myLogger.mu.Lock() and myLogger.withLock(fn) directly. +// The embedded withLock method is promoted, so code in this package can write +// myLogger.withLock(fn) directly. The embedded mu field is also promoted, but +// because it is unexported it is only directly accessible within the logger package. type lockable struct { mu sync.Mutex }