From 8c88aad06e27b8468a626a90a3420521b337ec62 Mon Sep 17 00:00:00 2001 From: Suriyakumarvijayanayagam Date: Fri, 18 Sep 2026 11:06:49 +0530 Subject: [PATCH] Stopping the engine on Windows stops the whole engine The installer runs the engine as \Scripts\python.exe, and since Python 3.7.2 that file is a redirector that spawns the real interpreter as a child. Stop() terminated the redirector and left the interpreter - the process holding the cameras and the SQLite WAL - running with no parent and nothing able to stop it. Seen on a Windows install: Quit from the tray, and recognition still running. The child is now started suspended, placed in a job object with KILL_ON_JOB_CLOSE, and resumed. Cancel terminates the job, so the whole tree goes; and the job dies with this process, so it goes even if the app crashes. CREATE_NO_WINDOW while here: python.exe is a console program and a GUI parent otherwise opens a black console on the shop counter. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KGcjxF1cNLcuwc3DAPcnfj --- agent/go.mod | 1 + agent/go.sum | 2 + agent/pkg/engine/supervisor.go | 20 ++++++ agent/pkg/engine/tree_other.go | 13 ++++ agent/pkg/engine/tree_windows.go | 111 +++++++++++++++++++++++++++++++ 5 files changed, 147 insertions(+) create mode 100644 agent/pkg/engine/tree_other.go create mode 100644 agent/pkg/engine/tree_windows.go diff --git a/agent/go.mod b/agent/go.mod index b0e925b..2fa92c6 100644 --- a/agent/go.mod +++ b/agent/go.mod @@ -7,4 +7,5 @@ require ( github.com/gorilla/websocket v1.5.0 // indirect golang.org/x/net v0.8.0 // indirect golang.org/x/sync v0.1.0 // indirect + golang.org/x/sys v0.20.0 // indirect ) diff --git a/agent/go.sum b/agent/go.sum index cf663d6..d0b9127 100644 --- a/agent/go.sum +++ b/agent/go.sum @@ -6,3 +6,5 @@ golang.org/x/net v0.8.0 h1:Zrh2ngAOFYneWTAIAPethzeaQLuHwhuBkuV6ZiRnUaQ= golang.org/x/net v0.8.0/go.mod h1:QVkue5JL9kW//ek3r6jTKnTFis1tRmNAW2P1shuFdJc= golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y= +golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= diff --git a/agent/pkg/engine/supervisor.go b/agent/pkg/engine/supervisor.go index fb0e37c..144d161 100644 --- a/agent/pkg/engine/supervisor.go +++ b/agent/pkg/engine/supervisor.go @@ -194,9 +194,29 @@ func (s *Supervisor) runOnce(ctx context.Context) error { return err } cmd.Stderr = cmd.Stdout + // Cancel ends the whole process tree, not just the process exec spawned. + // `kill` is filled in after Start, once the tree is confined; until then + // it is exec's own behaviour. + var kill func() error + cmd.Cancel = func() error { + if kill == nil { + return cmd.Process.Kill() + } + return kill() + } + prepare(cmd) if err := cmd.Start(); err != nil { return fmt.Errorf("engine failed to start: %w", err) } + k, release, err := confine(cmd) + if err != nil { + // Not fatal: the engine runs, and stopping it falls back to killing + // the one process. Logged because on Windows that fallback is the + // bug this exists to fix. + fmt.Fprintf(s.opts.LogWriter, "supervisor: could not confine engine process tree: %v\n", err) + } + kill = k + defer release() pumped := make(chan struct{}) go func() { diff --git a/agent/pkg/engine/tree_other.go b/agent/pkg/engine/tree_other.go new file mode 100644 index 0000000..1d6788f --- /dev/null +++ b/agent/pkg/engine/tree_other.go @@ -0,0 +1,13 @@ +//go:build !windows + +package engine + +import "os/exec" + +// On every other platform the engine is one process and exec's own kill is +// enough. See tree_windows.go for why Windows is not. +func prepare(*exec.Cmd) {} + +func confine(cmd *exec.Cmd) (kill func() error, release func(), err error) { + return cmd.Process.Kill, func() {}, nil +} diff --git a/agent/pkg/engine/tree_windows.go b/agent/pkg/engine/tree_windows.go new file mode 100644 index 0000000..fe6ed19 --- /dev/null +++ b/agent/pkg/engine/tree_windows.go @@ -0,0 +1,111 @@ +//go:build windows + +package engine + +import ( + "fmt" + "os/exec" + "syscall" + "unsafe" + + "golang.org/x/sys/windows" +) + +// The engine is not one process on Windows, and stopping it used to leave +// recognition running. +// +// The installer starts it as `\Scripts\python.exe -m behavision run`. +// Since Python 3.7.2 that python.exe is a REDIRECTOR: a small launcher that +// spawns the base interpreter as a child and waits for it. Stop() cancelled the +// context, exec terminated the launcher, and the interpreter that actually +// holds the cameras and the SQLite WAL carried on with no parent, no tray icon +// and nothing left that could stop it. Seen on a Windows install: "Quit +// Behavision" from the tray, and the engine still running. +// +// The fix is the primitive Windows has for exactly this: a job object with +// KILL_ON_JOB_CLOSE. Every process the engine spawns inherits membership, and +// the whole tree dies when the job is terminated or when this process's last +// handle to it goes away - so "quitting the app stops recognition" holds even +// if the app crashes, which no amount of careful Stop() code can promise. +// +// The child is started SUSPENDED and resumed only after it is in the job. +// Assigning after the fact leaves a window in which the launcher has already +// spawned the interpreter outside it, and that window is precisely the case +// this file exists to close. + +// prepare is applied to the command before it starts. +func prepare(cmd *exec.Cmd) { + if cmd.SysProcAttr == nil { + cmd.SysProcAttr = &syscall.SysProcAttr{} + } + // CREATE_NO_WINDOW: python.exe is a console program and Behavision.exe is + // not, so without this Windows opens a black console window for the + // engine on a shop counter - the app looks like it has crashed into a + // terminal. Output still arrives on the pipes. + cmd.SysProcAttr.CreationFlags |= windows.CREATE_SUSPENDED | windows.CREATE_NO_WINDOW +} + +// confine is applied after Start. It puts the process in a kill-on-close job, +// then resumes it. It returns a function that ends the whole tree, and one +// that releases the job handle once the tree has exited. +// +// If the job cannot be set up the process is still resumed and the plain +// terminate remains: a suspended engine that never runs is strictly worse +// than one that may outlive its parent. +func confine(cmd *exec.Cmd) (kill func() error, release func(), err error) { + pid := uint32(cmd.Process.Pid) + defer resumeProcess(pid) + + kill = cmd.Process.Kill + release = func() {} + + job, err := windows.CreateJobObject(nil, nil) + if err != nil { + return kill, release, fmt.Errorf("create job object: %w", err) + } + info := windows.JOBOBJECT_EXTENDED_LIMIT_INFORMATION{} + info.BasicLimitInformation.LimitFlags = windows.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE + if _, err := windows.SetInformationJobObject(job, windows.JobObjectExtendedLimitInformation, + uintptr(unsafe.Pointer(&info)), uint32(unsafe.Sizeof(info))); err != nil { + windows.CloseHandle(job) + return kill, release, fmt.Errorf("configure job object: %w", err) + } + proc, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE, false, pid) + if err != nil { + windows.CloseHandle(job) + return kill, release, fmt.Errorf("open engine process: %w", err) + } + defer windows.CloseHandle(proc) + if err := windows.AssignProcessToJobObject(job, proc); err != nil { + windows.CloseHandle(job) + return kill, release, fmt.Errorf("assign engine to job: %w", err) + } + + kill = func() error { return windows.TerminateJobObject(job, 1) } + release = func() { windows.CloseHandle(job) } + return kill, release, nil +} + +// resumeProcess resumes every thread of a process started CREATE_SUSPENDED. +// exec does not hand back the main thread handle, so it is found through the +// toolhelp snapshot; a suspended new process has exactly one. +func resumeProcess(pid uint32) { + snap, err := windows.CreateToolhelp32Snapshot(windows.TH32CS_SNAPTHREAD, 0) + if err != nil { + return + } + defer windows.CloseHandle(snap) + var te windows.ThreadEntry32 + te.Size = uint32(unsafe.Sizeof(te)) + for err = windows.Thread32First(snap, &te); err == nil; err = windows.Thread32Next(snap, &te) { + if te.OwnerProcessID != pid { + continue + } + h, err := windows.OpenThread(windows.THREAD_SUSPEND_RESUME, false, te.ThreadID) + if err != nil { + continue + } + windows.ResumeThread(h) + windows.CloseHandle(h) + } +}