This is an automated email from the ASF dual-hosted git repository. hsluoyz pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/casbin-gateway.git
commit 66068dacdc694227095ffa3559c45fa3fb594fef Author: Yang Luo <[email protected]> AuthorDate: Fri Sep 4 17:51:18 2026 +0800 fix: keep hook processes from leaking and name the port holder reliably --- agenthook/hook.go | 11 +++++++ util/port.go | 20 ++++--------- util/port_unix.go | 29 +++++++++++++++++++ util/port_windows.go | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 126 insertions(+), 15 deletions(-) diff --git a/agenthook/hook.go b/agenthook/hook.go index c6bccb0..b88e054 100644 --- a/agenthook/hook.go +++ b/agenthook/hook.go @@ -37,6 +37,13 @@ const ( maxHookInput = 8 * 1024 * 1024 reportTimeout = 5 * time.Second + // hookLifetime is how long the whole hook process may live. The report has + // its own timeout, but reading the event does not: an agent that starts a + // hook and then never writes the event, or never closes the pipe, leaves + // this process blocked in Decode forever. One copy of Gateway's executable + // per such event adds up - they are tens of megabytes each and only the + // machine's owner ever notices - so the process puts a deadline on itself. + hookLifetime = 30 * time.Second ) // ServeIfInvoked handles an agent-launched hook process before Gateway starts @@ -46,6 +53,10 @@ func ServeIfInvoked() { if len(os.Args) < 2 || os.Args[1] != Subcommand { return } + // Exit 0 either way: a hook that fails is a record not kept, never an agent + // action held up or refused. + timer := time.AfterFunc(hookLifetime, func() { os.Exit(0) }) + defer timer.Stop() _ = Run(os.Args[2:], os.Stdin) os.Exit(0) } diff --git a/util/port.go b/util/port.go index e324f97..2ca5349 100644 --- a/util/port.go +++ b/util/port.go @@ -29,8 +29,11 @@ import ( const ( // portLookupTimeout bounds the external "who owns this port" commands so a - // slow lsof or netstat can never hold up startup. - portLookupTimeout = 2 * time.Second + // slow lsof or netstat can never hold up startup. It is generous because a + // timeout here is not a slow answer but a wrong one: an unnamed holder is + // taken for a foreign program, and a previous Gateway that cannot be named + // is never stopped. + portLookupTimeout = 10 * time.Second // procNameMaxLen is how much of another process's name Linux reports, from // TASK_COMM_LEN. Anything longer comes back cut to this. procNameMaxLen = 15 @@ -219,19 +222,6 @@ func parseNetstatPid(output string, port int) int { return 0 } -// findProcessName resolves a pid to an executable name, returning "" when the -// platform command is missing or says nothing useful. -func findProcessName(pid int) string { - if runtime.GOOS == "windows" { - output := runLookup("tasklist", "/FI", fmt.Sprintf("PID eq %d", pid), "/NH", "/FO", "CSV") - // A CSV row looks like: "nginx.exe","1234","Console","1","5,000 K" - name := strings.TrimSpace(strings.SplitN(output, ",", 2)[0]) - return strings.Trim(name, `"`) - } - - return strings.TrimSpace(runLookup("ps", "-p", strconv.Itoa(pid), "-o", "comm=")) -} - // runLookup runs a short diagnostic command and returns its stdout, or "" on // any failure. Every caller treats the result as a hint, so errors are not // worth surfacing. diff --git a/util/port_unix.go b/util/port_unix.go new file mode 100644 index 0000000..2919141 --- /dev/null +++ b/util/port_unix.go @@ -0,0 +1,29 @@ +// Copyright 2026 The casbin Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build !windows + +package util + +import ( + "strconv" + "strings" +) + +// findProcessName resolves a pid to an executable name, returning "" when the +// platform command is missing or says nothing useful. "ps" is asked about the +// one pid, so it stays fast however many processes the host runs. +func findProcessName(pid int) string { + return strings.TrimSpace(runLookup("ps", "-p", strconv.Itoa(pid), "-o", "comm=")) +} diff --git a/util/port_windows.go b/util/port_windows.go new file mode 100644 index 0000000..3082260 --- /dev/null +++ b/util/port_windows.go @@ -0,0 +1,81 @@ +// Copyright 2026 The casbin Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package util + +import ( + "fmt" + "path/filepath" + "strings" + + "golang.org/x/sys/windows" +) + +// imagePathMaxLen is the buffer a full executable path is read into. Windows +// paths reach 32767 characters, but a process image path that long cannot be +// launched in the first place, so one MAX_PATH-sized try and one large one are +// all this ever needs. +const imagePathMaxLen = 32768 + +// findProcessName resolves a pid to an executable name such as "nginx.exe". +// +// The obvious way to do this on Windows is "tasklist", but tasklist enumerates +// and formats every process on the machine: on a host with a thousand of them +// it takes over two seconds, and a lookup that times out reports no name at +// all. That is not a cosmetic loss - an unnamed holder is treated as a foreign +// program, so "stop" refuses to stop Gateway's own server and a restart cannot +// reclaim its own port. Asking the kernel about the one pid we care about +// answers in microseconds and cannot be starved by an unrelated process count. +func findProcessName(pid int) string { + if name := processImageName(pid); name != "" { + return name + } + + // A process owned by another account can refuse to be opened. tasklist runs + // with the same rights, but it is worth one slow try before giving up. + output := runLookup("tasklist", "/FI", fmt.Sprintf("PID eq %d", pid), "/NH", "/FO", "CSV") + // A CSV row looks like: "nginx.exe","1234","Console","1","5,000 K" + name := strings.TrimSpace(strings.SplitN(output, ",", 2)[0]) + return strings.Trim(name, `"`) +} + +// processImageName reads one process's executable path straight from the +// kernel, and returns just its file name. It returns "" when the process is +// gone or this one may not query it. +func processImageName(pid int) string { + if pid <= 0 { + return "" + } + + // QUERY_LIMITED_INFORMATION is the least this can ask for and still read an + // image path, which is what lets it work across accounts. + handle, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid)) + if err != nil { + return "" + } + defer windows.CloseHandle(handle) + + buffer := make([]uint16, windows.MAX_PATH) + for { + size := uint32(len(buffer)) + err := windows.QueryFullProcessImageName(handle, 0, &buffer[0], &size) + if err == nil { + return filepath.Base(windows.UTF16ToString(buffer[:size])) + } + if err != windows.ERROR_INSUFFICIENT_BUFFER || len(buffer) >= imagePathMaxLen { + return "" + } + buffer = make([]uint16, imagePathMaxLen) + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
