The previous commit split arming from waiting so a caller could arm first,
wrote a comment saying a signal landing in between reaches the default
handler and kills the process, used it that way in the subprocess test - and
then left run() calling the combined helper after the whole readiness banner.
The window it warned about was still there in the one place that ships.
The signals are now armed before the server starts serving, and the wait
happens where it did. The disposition is restored right after the first
signal rather than deferred, so a shutdown that hangs can still be
interrupted by a second one.
waitForStopSignal goes away: run() was its only caller, and what was worth
keeping from its comment is now on armStopSignals.
Note that no test covers this ordering. The subprocess test drives
armStopSignals directly, which is what makes it a test of the mechanism
rather than of run(); moving the call back below the banner leaves it green.
Verified by reading the sequence in run(), not by a failing test.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
Three defects on one path, none of which could be seen from the code alone.
**SIGTERM was never registered.** signal.Notify listened for os.Interrupt
only, and Go terminates the process outright for a signal nothing handles.
`docker stop`, a Kubernetes pod deletion and `systemctl stop` all send
SIGTERM, so every line of the graceful shutdown below the wait was dead code
outside a terminal: measured on a real binary, SIGINT printed "Shutdown
Server ..." and "Server exiting" and SIGTERM printed neither.
**A stuck shutdown could not be interrupted.** quit is buffered and
signal.Notify stays armed after the first delivery, so further signals only
refill the buffer. That was harmless while SIGTERM went to the default
handler - it was the escape hatch. Registering it removes the hatch, so the
disposition is now restored once the first signal is taken, and a second one
kills the process the default way. Arming is split from waiting so a caller
can arm before it announces readiness; a signal in between reaches the
default handler, which is the very failure being fixed.
**A failed Shutdown skipped everything after it.** log.Fatal is an
unconditional os.Exit(1), and Shutdown reports an error precisely when
connections were still in flight - the moment the cleanup that follows
matters most. It is an error now, and the process carries on.
That failure is closer than it looks. net/http only treats a StateNew
connection as idle once it is over five seconds old, so a connection opened
shortly before the signal that has sent nothing holds the whole budget: with
the shipped settings.yml (readtimeout 1) the server closes it first and
shutdown takes 5ms, but with settings.demo.yml (readtimeout 10000) the same
connection made shutdown take 5.04s and exit 1, printing no "Server
exiting". The default configuration is what has been hiding this.
The wait and the shutdown are extracted so the subprocess tests can drive the
real functions against an empty http.Server: CI has no database, and none of
this needs one.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The package-level AppRouters slice keeps working and keeps running first, so
a fork that only ever appended to it sees no change. What is new is that the
core registry runs too, and that before callbacks run at all - this server
never had a loop for them.
Both go through core RunAppRouters / RunBefore, which brings the panic guard
and the seal with them.
Every import of the module changes, not only the seven packages that
moved out of sdk/pkg: Go requires the major version in the path from v2
on. Both happen in one pass —
go run github.com/go-admin-team/go-admin-core/tools/coreupgrade@v2.0.0 -w -v2 .
go mod tidy
— which is the command the release notes give, run here as a consumer
would run it. 210 imports across 95 files.
The compatibility shims this used are gone in v2, so the paths that
moved had to move: sdk/pkg/captcha, sdk/pkg/jwtauth and its user
package, sdk/pkg/response and sdk/pkg/casbin.
The count of unformatted files is unchanged at 34, none of them touched
by this: the tool reformats a file only if it was already gofmt clean,
so a migration cannot disappear into whitespace.
The pinned core dated from April, before sdk stopped being a separate module,
so the build resolved sdk packages from the old module and core packages from
the new one. Dropping the separate requirement is what makes the two agree
again.
Most of the diff is renames that came with that: the tenant accessors gained a
ByTenant suffix, GetDb now returns one database and GetAllDb the map, and
casbin moved to v3.
The change that matters is four call sites moving from GetMemoryQueue to
GetQueuePrefix. GetMemoryQueue returns a queue fixed at construction, so the
login log, the operate log and the api check ran in process no matter what the
settings file selected — a second instance saw none of it. GetQueuePrefix
returns whatever the configuration built, which is the point of being able to
configure a queue at all.
Verified against core at main: build and vet clean. The two file_store failures
are unchanged from before this branch; they need cloud credentials.
zhangwenjian