mirror of
https://github.com/kerberos-io/onvif.git
synced 2026-08-23 15:08:33 +00:00
70e6765a7d
Concurrency audit (third review) flagged that caller.SendSoap is not
ctx-aware: cancelling ctx does not unblock a pull or renew goroutine
parked in the underlying http.Client.Do. The previous Close()
unconditionally did <-s.done before its 5s unsubscribe timeout, so a
wedged SendSoap could hang Close indefinitely — taking the agent's
shutdown down with it.
Adds closeDrainTimeout (5s) to bound the wait for the run goroutines
to exit. When the drain times out:
* Close returns a 'did not drain' error so the caller can move on.
* Unsubscribe is skipped; the subscription expires at the camera
once InitialTermination elapses without a Renew.
* The wedged goroutines exit later, when the HTTP transport
eventually gives up. They are effectively leaked until then —
documented in the caller interface comment as the contract
callers must accept (or fix, by configuring an http.Client.Timeout).
The caller interface doc-comment now states both invariants
explicitly: must be goroutine-safe AND must enforce its own per-
request timeout, because we cannot from here.
Test
----
TestClose_BoundedWhenLoopsStuckOnHungHTTP: drives the fakeCaller
with blockAllSendSoap (new flag) so every SendSoap parks. Waits for
pullLoop to actually reach the blocked SendSoap before calling
Close (a race the previous attempt had: Close raced the loop and
exited via the ctx pre-check). Asserts Close returns within
closeDrainTimeout + 2s slack with a drain-timeout error.
Other concurrency audit findings disposition
--------------------------------------------
* unsubscribe goroutine leaks past 5s: intentional, already
documented at closeUnsubscribeTimeout.
* now func() time.Time data race: written once before goroutines
start; safe by happens-before. Tests do not swap it today.
* closeOnce self-deadlock if Close called from inside a loop:
no path exists; not exposed via the API.