Skip to content

Commit 97ab8fb

Browse files
authored
fix(guard): wazero improvements — shutdown cache cleanup, operational log routing, ReadUint32Le (#4700)
Three improvements to wazero usage in `internal/guard/wasm.go` identified by the Go Fan module review. ## Changes - **Compilation cache cleanup on shutdown**: `globalCompilationCache` was initialized at package startup but never closed in production. Added `guard.CloseGlobalCompilationCache(ctx)` and wired it into `InitiateShutdown` in `unified.go` after `guardRegistry.Close()`, so JIT resources are released on graceful shutdown. Failure is logged at `LogError` severity. ```go // internal/guard/wasm.go func CloseGlobalCompilationCache(ctx context.Context) error { return globalCompilationCache.Close(ctx) } // internal/server/unified.go — after guardRegistry.Close() if err := guard.CloseGlobalCompilationCache(context.Background()); err != nil { logger.LogError("shutdown", "Failed to close WASM compilation cache: %v", err) } ``` - **WASM warn/error → operational file logger**: `hostLog` previously sent all levels only to the debug logger (`logWasm`). `warn` and `error` now additionally call `logger.LogWarn`/`logger.LogError`, surfacing guard signals in `mcp-gateway.log` and `gateway.md`. - **Replace manual LE uint32 decode with `mem.ReadUint32Le()`**: Two duplicated bit-shift decodes in `tryCallWasmFunction` (allocator path and fallback path) replaced with the wazero typed helper, eliminating the intermediate byte-slice read. ```go // Before if sizeBytes, ok := mem.Read(outputPtr, 4); ok && len(sizeBytes) == 4 { requiredSize := uint32(sizeBytes[0]) | uint32(sizeBytes[1])<<8 | uint32(sizeBytes[2])<<16 | uint32(sizeBytes[3])<<24 // After if requiredSize, ok := mem.ReadUint32Le(outputPtr); ok && requiredSize > 0 { ``` > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### I tried to connect to the following addresses, but was blocked by firewall rules: > > - `example.com` > - Triggering command: `/tmp/go-build463346707/b513/launcher.test /tmp/go-build463346707/b513/launcher.test -test.testlogfile=/tmp/go-build463346707/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build463346707/b450/vet.cfg _.a elemetry.io/otel@v1.43.0/semconv/v1.40.0/doc.go x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet -I q8V2zTN1V -I x_amd64/vet --gdwarf-5 .io/otel/metric -o x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build192150560/b509/launcher.test /tmp/go-build192150560/b509/launcher.test -test.testlogfile=/tmp/go-build192150560/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libobject-926daa94a00ee327.rlib lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libmemchr-48d5b0db80402653.rlib lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libaddr2line-3367f26bd486b29d.rlib lib/rustlib/x86_go .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4�� .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o` (dns block) > - `invalid-host-that-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build463346707/b495/config.test /tmp/go-build463346707/b495/config.test -test.testlogfile=/tmp/go-build463346707/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build463346707/b393/vet.cfg 1.80.0/internal/resolver/config_selector.go 236262/b151/ x_amd64/vet --gdwarf-5 go-sdk/internal/-atomic -o x_amd64/vet 2362�� _.a ache/go/1.25.9/x-ifaceassert x_amd64/vet 236262/b151/ --64` (dns block) > - Triggering command: `/tmp/go-build192150560/b491/config.test /tmp/go-build192150560/b491/config.test -test.testlogfile=/tmp/go-build192150560/b491/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true -guard/target/de/home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/degit -guard/target/de/home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/de-c -guard/target/de/home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/delog.showsignature=false ntime.v2.task/mogit f07e34dca1.buildrev-parse lib/rustlib/x86_HEAD lib/rustlib/x86_/home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/de--format=format:%H %ct %D lib/�� lib/rustlib/x86_/home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/de/opt/hostedtoolcache/go/1.25.9/x64/pkg/tool/linux_amd64/vet lib/rustlib/x86_/home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/de/tmp/go-build2805575210/b498/vet.cfg lib/rustlib/x86_/home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/debug/deps/github_guard-57d41235e07a5585.3ky4jndxwogptk4p3m4cqblia.0y4p35u.rcgu.o lib/rustlib/x86_/opt/hostedtoolcache/CodeQL/2.25.1/x64/codeql/codeql lib/rustlib/x86_database lib/rustlib/x86_analyze lib/rustlib/x86_--ram=14575` (dns block) > - `nonexistent.local` > - Triggering command: `/tmp/go-build463346707/b513/launcher.test /tmp/go-build463346707/b513/launcher.test -test.testlogfile=/tmp/go-build463346707/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build463346707/b450/vet.cfg _.a elemetry.io/otel@v1.43.0/semconv/v1.40.0/doc.go x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet -I q8V2zTN1V -I x_amd64/vet --gdwarf-5 .io/otel/metric -o x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build192150560/b509/launcher.test /tmp/go-build192150560/b509/launcher.test -test.testlogfile=/tmp/go-build192150560/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libobject-926daa94a00ee327.rlib lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libmemchr-48d5b0db80402653.rlib lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libaddr2line-3367f26bd486b29d.rlib lib/rustlib/x86_go .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4�� .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o` (dns block) > - `slow.example.com` > - Triggering command: `/tmp/go-build463346707/b513/launcher.test /tmp/go-build463346707/b513/launcher.test -test.testlogfile=/tmp/go-build463346707/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build463346707/b450/vet.cfg _.a elemetry.io/otel@v1.43.0/semconv/v1.40.0/doc.go x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet -I q8V2zTN1V -I x_amd64/vet --gdwarf-5 .io/otel/metric -o x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build192150560/b509/launcher.test /tmp/go-build192150560/b509/launcher.test -test.testlogfile=/tmp/go-build192150560/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libobject-926daa94a00ee327.rlib lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libmemchr-48d5b0db80402653.rlib lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libaddr2line-3367f26bd486b29d.rlib lib/rustlib/x86_go .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4�� .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o .0y4p35u.rcgu.o` (dns block) > - `this-host-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build463346707/b522/mcp.test /tmp/go-build463346707/b522/mcp.test -test.testlogfile=/tmp/go-build463346707/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s 2362�� .cfg om/tetratelabs/w-ifaceassert x_amd64/vet . --gdwarf2 --64 x_amd64/vet .cfg�� 236262/b381/_pkg_.a -fPIC x_amd64/vet -pthread g/grpc/experimen--version -fmessage-length=0 x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build192150560/b518/mcp.test /tmp/go-build192150560/b518/mcp.test -test.testlogfile=/tmp/go-build192150560/b518/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true st-guard/target/debug/deps/rustc0bKEfQ/raw-dylibs u/lib/rustlib/x86_64-REDACTED-linux-gnu/lib known-linux-gnu/lib/rustlib/x86_64-REDACTED-linux-gnu/bin/rust-lld ithub-guard/rustbash cf4be8585821fe3f/usr/bin/runc ild known-linux-gnu/lib/rustlib/x86_diff know�� known-linux-gnu/lib/rustlib/x86_--irreversible-delete ild in.so /lto-wrapper known-linux-gnu/info known-linux-gnu/lib/rustlib/x86_log.showsignature=false known-linux-gnu/lib/rustlib/x86_log` (dns block) > > If you need me to access, download, or install something from one of these locations, you can either: > > - Configure [Actions setup steps](https://gh.io/copilot/actions-setup-steps) to set up my environment, which run before the firewall is enabled > - Add the appropriate URLs or hosts to the custom allowlist in this repository's [Copilot coding agent settings](https://github.com/github/gh-aw-mcpg/settings/copilot/coding_agent) (admins only) > > </details>
2 parents daf15b6 + 59e2235 commit 97ab8fb

2 files changed

Lines changed: 20 additions & 10 deletions

File tree

internal/guard/wasm.go

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,15 @@ var logWasm = logger.New("guard:wasm")
2525
// JIT compilation when multiple guards load the same WASM binary.
2626
var globalCompilationCache = wazero.NewCompilationCache()
2727

28+
// CloseGlobalCompilationCache releases JIT resources held by the shared
29+
// compilation cache. It must be called once during graceful shutdown, after
30+
// all WasmGuard runtimes have been closed (i.e., after Registry.Close()).
31+
// Calling it while guards are still active or calling it more than once leads
32+
// to undefined behavior. It is not safe to call concurrently.
33+
func CloseGlobalCompilationCache(ctx context.Context) error {
34+
return globalCompilationCache.Close(ctx)
35+
}
36+
2837
// WasmGuardOptions configures optional settings for WASM guard creation
2938
type WasmGuardOptions struct {
3039
// Stdout is the writer for WASM stdout output. Defaults to os.Stdout if nil.
@@ -310,8 +319,10 @@ func (g *WasmGuard) hostLog(ctx context.Context, m api.Module, stack []uint64) {
310319
logWasm.Printf("%sINFO: %s", prefix, msg)
311320
case logLevelWarn:
312321
logWasm.Printf("%sWARN: %s", prefix, msg)
322+
logger.LogWarn("guard", "[%s] %s", g.name, msg)
313323
case logLevelError:
314324
logWasm.Printf("%sERROR: %s", prefix, msg)
325+
logger.LogError("guard", "[%s] %s", g.name, msg)
315326
default:
316327
logWasm.Printf("%s%s", prefix, msg)
317328
}
@@ -972,11 +983,8 @@ func (g *WasmGuard) tryCallWasmFunction(ctx context.Context, fn api.Function, me
972983

973984
resultLen := int32(results[0])
974985
if resultLen == -2 {
975-
if sizeBytes, ok := mem.Read(outputPtr, 4); ok && len(sizeBytes) == 4 {
976-
requiredSize := uint32(sizeBytes[0]) | uint32(sizeBytes[1])<<8 | uint32(sizeBytes[2])<<16 | uint32(sizeBytes[3])<<24
977-
if requiredSize > 0 {
978-
return nil, requiredSize, nil
979-
}
986+
if requiredSize, ok := mem.ReadUint32Le(outputPtr); ok && requiredSize > 0 {
987+
return nil, requiredSize, nil
980988
}
981989
return nil, 0, nil
982990
}
@@ -1039,11 +1047,8 @@ func (g *WasmGuard) tryCallWasmFunction(ctx context.Context, fn api.Function, me
10391047
// The guard can optionally return the required size in the output buffer as a uint32
10401048
if resultLen == -2 {
10411049
// Try to read the required size from the output buffer (first 4 bytes as uint32)
1042-
if sizeBytes, ok := mem.Read(outputPtr, 4); ok && len(sizeBytes) == 4 {
1043-
requiredSize := uint32(sizeBytes[0]) | uint32(sizeBytes[1])<<8 | uint32(sizeBytes[2])<<16 | uint32(sizeBytes[3])<<24
1044-
if requiredSize > 0 {
1045-
return nil, requiredSize, nil
1046-
}
1050+
if requiredSize, ok := mem.ReadUint32Le(outputPtr); ok && requiredSize > 0 {
1051+
return nil, requiredSize, nil
10471052
}
10481053
// Guard didn't specify size, return 0 to trigger doubling
10491054
return nil, 0, nil

internal/server/unified.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -809,6 +809,11 @@ func (us *UnifiedServer) InitiateShutdown() int {
809809
us.guardRegistry.Close(context.Background())
810810
}
811811

812+
// Release JIT resources held by the shared WASM compilation cache
813+
if err := guard.CloseGlobalCompilationCache(context.Background()); err != nil {
814+
logger.LogError("shutdown", "Failed to close WASM compilation cache: %v", err)
815+
}
816+
812817
logger.LogInfo("shutdown", "Backend servers terminated successfully")
813818
})
814819
return serversTerminated

0 commit comments

Comments
 (0)