From 20bd3081adffa3b70206c2c5138bb776988361b4 Mon Sep 17 00:00:00 2001 From: Rajeh Taher Date: Thu, 22 Oct 2026 05:39:08 +0300 Subject: [PATCH 01/10] Add fail-closed final client shutdown and core worker joins --- CLIENT_LIFETIME.md | 75 ++++++++ client.go | 95 +++++++++-- client_lifecycle.go | 198 +++++++++++++++++++++ client_lifecycle_test.go | 360 +++++++++++++++++++++++++++++++++++++++ keepalive.go | 10 +- socket/framesocket.go | 36 +++- socket/lifecycle_test.go | 111 ++++++++++++ socket/noisesocket.go | 10 +- 8 files changed, 867 insertions(+), 28 deletions(-) create mode 100644 CLIENT_LIFETIME.md create mode 100644 client_lifecycle.go create mode 100644 client_lifecycle_test.go create mode 100644 socket/lifecycle_test.go diff --git a/CLIENT_LIFETIME.md b/CLIENT_LIFETIME.md new file mode 100644 index 000000000..f3f887bec --- /dev/null +++ b/CLIENT_LIFETIME.md @@ -0,0 +1,75 @@ +# Final client close + +`(*Client).FinalClose(ctx)` permanently seals one exact Client instance. Pass a +context with a deadline. `Disconnect` keeps its reusable behavior; it does not +seal the instance. Neither operation logs out or deletes the device store. + +This is local lifecycle source, not a published contract or accepted deployment +producer. **FinalClose never returns nil.** Once the tracked subset joins it +returns `ErrClientShutdownIncomplete`. Context cancellation/deadline errors mean +that this waiter did not observe even that subset's completion. A cancelled +invocation before sealing makes no lifecycle change. A later invocation joins +the same teardown, rather than retrying it. No stop receipt may be derived from +any of these errors, a socket disconnect, or a cancelled context. + +Concurrent callers share one teardown of the same pointer. Replacement Client +instances have separate lifetimes. Do not copy Client values. No client or socket +state lock is held across a completion wait. A blocked external transport or +callback can keep the owned teardown alive after a caller's deadline. Calling +FinalClose from a callback cannot finish joining that callback until it returns; +the deadline ends its waiter and does not manufacture completion. + +## Tracked native subset + +- ConnectContext and internal reconnect attempts, including admitted dials and + handshakes. Final close interrupts their contexts and refuses future attempts. +- Keepalive and handler-queue loops, connection-triggered disconnected/reconnect + tasks, handler-queue reset tasks, and each scheduled node handler. +- Synchronous receive processing, RawNodeHandler, dispatchEvent and each admitted + event handler, plus AutoReconnectHook invoked by tracked reconnect work. +- FrameSocket read pump, NoiseSocket consume loop, socket disconnect callbacks, + asynchronous socket close, and asynchronous writes. Client installs the socket + launcher before dialing. The launcher records owned completion before scheduling. + A write remains tracked after SendFrame returns on context cancellation. +- One teardown task, whose completion is recorded before launch. Context forwarding + uses context.AfterFunc only to cancel contexts, with no user/protocol callbacks; + those small cancellation callbacks are not a joined product-worker claim. + +A permanent seal prevents new admissions to this subset. Work admitted before +sealing may execute after sealing and remains counted until its function returns. +Frame delivery can exit when the connection context is cancelled, rather than +remaining stuck sending to an abandoned consumer. Socket ciphertext, counters, +keys, handshake and wire formats are unchanged. Standalone FrameSocket users keep +the default launcher unless they explicitly install their own before Connect. + +## Still unbound + +The following existing client-owned producers retain their prior behavior and +are NOT included in the completion claim. Some use BackgroundEventCtx or +WithoutCancel and can continue launching child work after the core seal. This +is why the implementation cannot confirm full termination even when its tracked +count reaches zero. Their admission, cancellation and actual completion need +owning-library work before any successful stop result is possible. + +| Source at base 29a42459 | Unbound lifetime | +| --- | --- | +| connectionevents.go:33,187 | Stream cleanup, connect-success/prekey/upload/session work and async event-dispatch launch wrappers. Event callback invocation itself is sealed by dispatchEvent, but the wrapper goroutine is unbound. | +| notification.go:57,522-566,603 | Privacy token, encryption/app-state/account-sync, code/passkey pairing, and server AB-props sync tasks. | +| message.go:82-89,373-374,469-470,520,549,770,887,1005-1037 | Name updates, retry/ack work, event-buffer cleanup, event-dispatch wrapper, history download/storage/notification loops, key-share and peer-data work. | +| appstate.go:162,645,659 | Missing-key request and async event batches. | +| pair.go:138-147 | Pair completion, PrePairCallback and unified-session child task. | +| receipt.go:27,36,109,128; retry.go:469,541 | Async ack/retry handlers and delayed phone rerequests. Cancellation of delayed requests is not their join. | +| presence.go:71; send.go:903; tctoken.go:150 | Unified-session, privacy-token persistence and expired-token pruning. | +| qrchan.go:80,102,119,135,195 | QR emission and async handler removals. | +| handshake.go:98-103 | GetClientPayload callback admission is not separately sealed; an admitted connection can reach it. The enclosing connect is counted, which is not full callback-admission coverage. | + +Caller-invoked public API methods, DangerousInternals, external HTTP transports, +user store implementations and goroutines launched by user callbacks remain +separate caller-owned work. Their owner must stop and join them separately. +The socket wrapper tracks HyperMeow's explicit goroutines; it does not assert +joined internals for the third-party websocket/HTTP transport. No Signal +protocol, identity, state, crypto, custody, funding or placement authority is +added by this lifecycle source. + +API integration, exact client-incarnation custody, runner producers, WAM, +BanSafe, Calls, Region and full deployment composition remain separate gates. diff --git a/client.go b/client.go index fea52388d..74298499a 100644 --- a/client.go +++ b/client.go @@ -93,6 +93,8 @@ type deviceCache struct { // Client contains everything necessary to connect to and interact with the WhatsApp web API. type Client struct { + lifetime clientLifetime + Store *store.Device Log waLog.Logger recvLog waLog.Logger @@ -584,20 +586,35 @@ func (cli *Client) ConnectContext(ctx context.Context) error { return ErrClientIsNil } + done, ok := cli.enterNative() + if !ok { + return ErrClientClosed + } + defer done() + ctx = cli.nativeContext(ctx) cli.socketLock.Lock() defer cli.socketLock.Unlock() err := cli.unlockedConnect(ctx) + if cli.nativeClosed() { + return ErrClientClosed + } if isRetryableConnectError(err) && cli.InitialAutoReconnect && cli.EnableAutoReconnect { cli.Log.Errorf("Initial connection failed but reconnecting in background (%v)", err) - go cli.dispatchEvent(&events.Disconnected{}) - go cli.autoReconnect(ctx) + cli.startNative(func() { cli.dispatchEvent(&events.Disconnected{}) }) + cli.startNative(func() { cli.autoReconnect(ctx) }) return nil } return err } func (cli *Client) connect(ctx context.Context) error { + done, ok := cli.enterNative() + if !ok { + return ErrClientClosed + } + defer done() + ctx = cli.nativeContext(ctx) cli.socketLock.Lock() defer cli.socketLock.Unlock() @@ -605,6 +622,9 @@ func (cli *Client) connect(ctx context.Context) error { } func (cli *Client) unlockedConnect(ctx context.Context) error { + if cli.nativeClosed() { + return ErrClientClosed + } if cli.shadowRelay != nil { // Guard: a shadow client must never open a socket. All Connect // paths (Connect, ConnectContext, autoReconnect) funnel through @@ -628,6 +648,8 @@ func (cli *Client) unlockedConnect(ctx context.Context) error { client = cli.preLoginHTTP } fs := socket.NewFrameSocket(cli.Log.Sub("Socket"), client) + releaseSocket := cli.bindSocket(fs) + defer releaseSocket() if userAgent := cli.getUserAgent(); userAgent != "" { fs.HTTPHeaders.Set("User-Agent", userAgent) } @@ -651,8 +673,8 @@ func (cli *Client) unlockedConnect(ctx context.Context) error { fs.Close(0) return fmt.Errorf("noise handshake failed: %w", err) } - go cli.keepAliveLoop(ctx, fs.Context()) - go cli.handlerQueueLoop(ctx, fs.Context()) + cli.startNative(func() { cli.keepAliveLoop(ctx, fs.Context()) }) + cli.startNative(func() { cli.handlerQueueLoop(ctx, fs.Context()) }) return nil } @@ -670,8 +692,8 @@ func (cli *Client) onDisconnect(ctx context.Context, ns *socket.NoiseSocket, rem cli.clearResponseWaiters(xmlStreamEndNode) if !cli.isExpectedDisconnect() && (cli.forceAutoReconnect.Swap(false) || remote) { cli.Log.Debugf("Emitting Disconnected event") - go cli.dispatchEvent(&events.Disconnected{}) - go cli.autoReconnect(ctx) + cli.startNative(func() { cli.dispatchEvent(&events.Disconnected{}) }) + cli.startNative(func() { cli.autoReconnect(ctx) }) } else if remote { cli.Log.Debugf("OnDisconnect() called, but it was expected, so not emitting event") } else { @@ -697,6 +719,9 @@ func (cli *Client) isExpectedDisconnect() bool { } func (cli *Client) autoReconnect(ctx context.Context) { + if cli.nativeClosed() { + return + } if !cli.EnableAutoReconnect || cli.Store.ID == nil { return } @@ -712,6 +737,9 @@ func (cli *Client) autoReconnect(ctx context.Context) { return } err := cli.connect(ctx) + if errors.Is(err, ErrClientClosed) || cli.nativeClosed() { + return + } if errors.Is(err, ErrAlreadyConnected) { cli.Log.Debugf("Connect() said we're already connected after autoreconnect sleep") return @@ -721,9 +749,16 @@ func (cli *Client) autoReconnect(ctx context.Context) { return } cli.Log.Errorf("Error reconnecting after autoreconnect sleep: %v", err) - if cli.AutoReconnectHook != nil && !cli.AutoReconnectHook(err) { - cli.Log.Debugf("AutoReconnectHook returned false, not reconnecting") - return + if hook := cli.AutoReconnectHook; hook != nil { + doneCallback, admitted := cli.enterNative() + if !admitted { + return + } + retry := func() bool { defer doneCallback(); return hook(err) }() + if !retry { + cli.Log.Debugf("AutoReconnectHook returned false, not reconnecting") + return + } } } else { return @@ -761,7 +796,7 @@ func (cli *Client) Disconnect() { // ResetConnection disconnects from the WhatsApp web websocket and forces an automatic reconnection. // This will not do anything if the socket is already disconnected or if EnableAutoReconnect is false. func (cli *Client) ResetConnection() { - if cli == nil { + if cli == nil || cli.nativeClosed() { return } cli.socketLock.Lock() @@ -908,6 +943,11 @@ func (cli *Client) RemoveEventHandlers() { } func (cli *Client) handleFrame(ctx context.Context, data []byte) { + done, ok := cli.enterNative() + if !ok { + return + } + defer done() decompressed, err := waBinary.Unpack(data) if err != nil { cli.Log.Warnf("Failed to decompress frame: %v", err) @@ -921,7 +961,14 @@ func (cli *Client) handleFrame(ctx context.Context, data []byte) { return } if h := cli.RawNodeHandler; h != nil { - modified, drop := h(ctx, node) + doneCallback, admitted := cli.enterNative() + if !admitted { + return + } + modified, drop := func() (*waBinary.Node, bool) { + defer doneCallback() + return h(ctx, node) + }() if drop { cli.recvLog.Debugf("RawNodeHandler dropped node: %s", node) return @@ -978,13 +1025,15 @@ func (cli *Client) enqueueNode(ctx context.Context, node *waBinary.Node) { default: if cli.forceAutoReconnect.CompareAndSwap(false, true) { cli.Log.Errorf("Handler queue is full, resetting connection") - go cli.ResetConnection() + cli.startNative(func() { cli.ResetConnection() }) } } } func (cli *Client) handlerQueueLoop(evtCtx, connCtx context.Context) { + finalDone := cli.nativeDone() ticker := time.NewTicker(30 * time.Second) + defer ticker.Stop() ticker.Stop() cli.Log.Debugf("Starting handler queue loop") Loop: @@ -993,26 +1042,32 @@ Loop: case node := <-cli.handlerQueue: doneChan := make(chan struct{}) start := time.Now() - go func() { + if !cli.startNative(func() { cli.handleNode(evtCtx, node) duration := time.Since(start) close(doneChan) if duration > 5*time.Second { cli.Log.Warnf("Node handling took %s for %s", duration, node) } - }() + }) { + return + } ticker.Reset(30 * time.Second) for i := 0; i < 10; i++ { select { case <-doneChan: ticker.Stop() continue Loop + case <-finalDone: + return case <-ticker.C: cli.Log.Warnf("Node handling is taking long for %s (started %s ago)", node, time.Since(start)) } } cli.Log.Warnf("Continuing handling of %s in background as it's taking too long", node) ticker.Stop() + case <-finalDone: + return case <-connCtx.Done(): cli.Log.Debugf("Closing handler queue loop") return @@ -1090,6 +1145,11 @@ func (cli *Client) sendNode(ctx context.Context, node waBinary.Node) error { } func (cli *Client) dispatchEvent(evt any) (handlerFailed bool) { + done, ok := cli.enterNative() + if !ok { + return true + } + defer done() cli.eventHandlersLock.RLock() defer func() { cli.eventHandlersLock.RUnlock() @@ -1099,7 +1159,12 @@ func (cli *Client) dispatchEvent(evt any) (handlerFailed bool) { } }() for _, handler := range cli.eventHandlers { - if !handler.fn(evt) { + doneCallback, admitted := cli.enterNative() + if !admitted { + return true + } + success := func() bool { defer doneCallback(); return handler.fn(evt) }() + if !success { return true } } diff --git a/client_lifecycle.go b/client_lifecycle.go new file mode 100644 index 000000000..1ccdcf19b --- /dev/null +++ b/client_lifecycle.go @@ -0,0 +1,198 @@ +package whatsmeow + +import ( + "context" + "errors" + "sync" + + "github.com/polymorfa/hypermeow/socket" +) + +var ( + // ErrClientClosed means FinalClose permanently sealed this Client instance. + ErrClientClosed = errors.New("client lifetime is closed") + // ErrClientShutdownIncomplete means the tracked native subset joined, but + // other client-owned async producers do not yet have a joined lifetime. + // It is never confirmation of client termination or a deployment stop receipt. + ErrClientShutdownIncomplete = errors.New("client shutdown coverage is incomplete") + // ErrShutdownDeadlineRequired prevents an unbounded wait, including from a callback. + ErrShutdownDeadlineRequired = errors.New("client shutdown requires a deadline") +) + +type clientLifetime struct { + mu sync.Mutex + ctx context.Context + cancel context.CancelFunc + sealed bool + active int + joined chan struct{} + stopped chan struct{} + sockets map[*socket.FrameSocket]int +} + +func (l *clientLifetime) initLocked() { + if l.ctx == nil { + l.ctx, l.cancel = context.WithCancel(context.Background()) + l.joined = make(chan struct{}) + l.sockets = make(map[*socket.FrameSocket]int) + } +} + +// Admission and completion share one lock. No Add can race a completed join. +func (cli *Client) enterNative() (func(), bool) { + l := &cli.lifetime + l.mu.Lock() + l.initLocked() + if l.sealed { + l.mu.Unlock() + return nil, false + } + l.active++ + l.mu.Unlock() + return func() { + l.mu.Lock() + l.active-- + if l.sealed && l.active == 0 { + close(l.joined) + } + l.mu.Unlock() + }, true +} + +func (cli *Client) startNative(fn func()) bool { + done, ok := cli.enterNative() + if !ok { + return false + } + go func() { defer done(); fn() }() + return true +} + +func (cli *Client) nativeDone() <-chan struct{} { + l := &cli.lifetime + l.mu.Lock() + defer l.mu.Unlock() + l.initLocked() + return l.ctx.Done() +} + +func (cli *Client) nativeClosed() bool { + l := &cli.lifetime + l.mu.Lock() + defer l.mu.Unlock() + return l.sealed +} + +// Socket workers include read/receive, disconnect callbacks, async close and +// writes that outlive SendFrame cancellation. Register before scheduling. +func (cli *Client) bindSocket(fs *socket.FrameSocket) func() { + l := &cli.lifetime + l.mu.Lock() + l.initLocked() + l.sockets[fs]++ // Keep the in-progress dial in the registry as well. + l.mu.Unlock() + release := func() { + l.mu.Lock() + l.sockets[fs]-- + if l.sockets[fs] == 0 && !fs.IsConnected() { + delete(l.sockets, fs) + } + l.mu.Unlock() + } + fs.SetWorkerLauncher(func(fn func()) bool { + done, ok := cli.enterNative() + if !ok { + return false + } + l.mu.Lock() + l.sockets[fs]++ + l.mu.Unlock() + go func() { defer done(); defer release(); fn() }() + return true + }) + return release +} + +// nativeContext retains the caller's cancellation and adds final-close +// cancellation. Its cancellation callback only cancels a context; it performs +// no user callback, socket operation or protocol work. +func (cli *Client) nativeContext(parent context.Context) context.Context { + l := &cli.lifetime + l.mu.Lock() + l.initLocked() + lifetime := l.ctx + l.mu.Unlock() + ctx, cancel := context.WithCancel(parent) + stop := context.AfterFunc(lifetime, cancel) + context.AfterFunc(ctx, func() { stop(); cancel() }) + return ctx +} + +// FinalClose permanently seals this exact Client pointer. Unlike Disconnect, +// it forbids reconnect and new core worker/callback admissions. It starts one +// teardown shared by concurrent callers; cancelling a waiter does not retry or +// cancel teardown. Callers must provide a deadline, especially inside callbacks: +// a callback cannot finish joining its own invocation before it returns. +// +// This implementation joins the core socket/connect/reconnect/handler/event +// subset only. It ALWAYS returns ErrClientShutdownIncomplete after that join. +// See CLIENT_LIFETIME.md for unbound async producers. Public operations started +// by another runtime owner and work spawned by user callbacks are caller-owned. +// No logout, store deletion, assignment or authority change is performed. +func (cli *Client) FinalClose(ctx context.Context) error { + if cli == nil { + return ErrClientIsNil + } + if ctx == nil { + return ErrShutdownDeadlineRequired + } + if err := ctx.Err(); err != nil { + return err + } + if _, ok := ctx.Deadline(); !ok { + return ErrShutdownDeadlineRequired + } + l := &cli.lifetime + l.mu.Lock() + l.initLocked() + if !l.sealed { + l.sealed = true + l.stopped = make(chan struct{}) // Own teardown completion before scheduling. + if l.active == 0 { + close(l.joined) + } + go cli.finalDisconnect() + } + stopped := l.stopped + l.mu.Unlock() + select { + case <-ctx.Done(): + return ctx.Err() + case <-stopped: + if err := ctx.Err(); err != nil { + return err + } + return ErrClientShutdownIncomplete + } +} + +func (cli *Client) finalDisconnect() { + // Cancellation is not a join. Keep even cancellation fanout outside the + // lifetime mutex and the bounded caller. + cli.lifetime.cancel() + // Disconnect may wait for a dial/handshake's socket lock. Never make a + // bounded caller perform that blocking work, or hold the lifetime lock here. + cli.Disconnect() + l := &cli.lifetime + l.mu.Lock() + sockets := make([]*socket.FrameSocket, 0, len(l.sockets)) + for fs := range l.sockets { + sockets = append(sockets, fs) + } + l.mu.Unlock() + for _, fs := range sockets { + fs.Close(0) + } + <-l.joined + close(l.stopped) +} diff --git a/client_lifecycle_test.go b/client_lifecycle_test.go new file mode 100644 index 000000000..481d09f1d --- /dev/null +++ b/client_lifecycle_test.go @@ -0,0 +1,360 @@ +package whatsmeow + +import ( + "context" + "errors" + "net/http" + "sync/atomic" + + waBinary "github.com/polymorfa/hypermeow/binary" + "github.com/polymorfa/hypermeow/types" + "testing" + "time" + + "github.com/polymorfa/hypermeow/store" +) + +func TestFinalCloseSealsExactClient(t *testing.T) { + cli := NewClient(&store.Device{}, nil) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + if err := cli.FinalClose(ctx); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("want incomplete coverage, got %v", err) + } + if err := cli.ConnectContext(ctx); !errors.Is(err, ErrClientClosed) { + t.Fatalf("connect after close: %v", err) + } + if err := cli.FinalClose(ctx); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("replay: %v", err) + } +} + +func finalTestContext(t *testing.T) context.Context { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + t.Cleanup(cancel) + return ctx +} + +func finalTestWait(t *testing.T, ch <-chan struct{}) { + t.Helper() + select { + case <-ch: + case <-time.After(2 * time.Second): + t.Fatal("test worker did not reach barrier") + } +} + +func finalTestBlocked(t *testing.T, cli *Client) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + sealed := cli.nativeDone() + result := make(chan error, 1) + go func() { result <- cli.FinalClose(ctx) }() + finalTestWait(t, sealed) + cancel() + if err := <-result; !errors.Is(err, context.Canceled) { + t.Fatalf("blocked lifetime must cancel, got %v", err) + } + cli.lifetime.mu.Lock() + stopped := cli.lifetime.stopped + cli.lifetime.mu.Unlock() + select { + case <-stopped: + t.Fatal("reported teardown completion while native work is blocked") + default: + } +} + +func TestFinalCloseWaitsCallbackAfterDisconnect(t *testing.T) { + cli := NewClient(&store.Device{}, nil) + entered, release, returned := make(chan struct{}), make(chan struct{}), make(chan struct{}) + defer func() { + select { + case <-release: + default: + close(release) + } + }() + cli.AddEventHandler(func(any) { close(entered); <-release }) + go func() { cli.dispatchEvent("synthetic event"); close(returned) }() + finalTestWait(t, entered) + cli.Disconnect() // A disconnected socket does not complete the admitted callback. + finalTestBlocked(t, cli) + if cli.startNative(func() { t.Error("late worker ran") }) { + t.Fatal("late admission accepted") + } + if !cli.dispatchEvent("late event") { + t.Fatal("late callback admission accepted") + } + close(release) + finalTestWait(t, returned) + if err := cli.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("join must retain coverage refusal: %v", err) + } +} + +func TestFinalCloseWaitsHandlerAfterLoopCancellation(t *testing.T) { + cli := NewClient(&store.Device{}, nil) + entered, release, loopDone := make(chan struct{}), make(chan struct{}), make(chan struct{}) + defer func() { + select { + case <-release: + default: + close(release) + } + }() + cli.nodeHandlers["lifetime_test"] = func(context.Context, *waBinary.Node) { close(entered); <-release } + connCtx, cancel := context.WithCancel(context.Background()) + defer cancel() + cli.startNative(func() { defer close(loopDone); cli.handlerQueueLoop(context.Background(), connCtx) }) + cli.handlerQueue <- &waBinary.Node{Tag: "lifetime_test"} + finalTestWait(t, entered) + cancel() + finalTestBlocked(t, cli) + finalTestWait(t, loopDone) + close(release) + if err := cli.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("handler join: %v", err) + } +} + +func TestFinalCloseConcurrentCancellationReplayAndReplacement(t *testing.T) { + cli := NewClient(&store.Device{}, nil) + entered, release := make(chan struct{}), make(chan struct{}) + defer func() { + select { + case <-release: + default: + close(release) + } + }() + cli.startNative(func() { close(entered); <-release }) + finalTestWait(t, entered) + finalTestBlocked(t, cli) + cli.lifetime.mu.Lock() + originalStop := cli.lifetime.stopped + cli.lifetime.mu.Unlock() + results := make(chan error, 4) + for i := 0; i < 4; i++ { + go func() { results <- cli.FinalClose(finalTestContext(t)) }() + } + cancelled, cancel := context.WithCancel(finalTestContext(t)) + cancel() + if err := cli.FinalClose(cancelled); !errors.Is(err, context.Canceled) { + t.Fatalf("cancelled waiter: %v", err) + } + replacement := NewClient(&store.Device{}, nil) + replacementDone := make(chan struct{}) + if !replacement.startNative(func() { close(replacementDone) }) { + t.Fatal("old stop sealed replacement") + } + finalTestWait(t, replacementDone) + close(release) + for i := 0; i < 4; i++ { + if err := <-results; !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("coalesced stop: %v", err) + } + } + cli.lifetime.mu.Lock() + sameStop := cli.lifetime.stopped == originalStop + cli.lifetime.mu.Unlock() + if !sameStop { + t.Fatal("stop retried teardown") + } + if replacement.nativeClosed() { + t.Fatal("replacement was stopped") + } +} + +func TestFinalCloseSelfCallbackRequiresOwnReturn(t *testing.T) { + cli := NewClient(&store.Device{}, nil) + result := make(chan error, 1) + cli.AddEventHandler(func(any) { + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + result <- cli.FinalClose(ctx) + }) + cli.startNative(func() { cli.dispatchEvent("self stop") }) + if err := <-result; !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("self callback must not confirm join: %v", err) + } + if !cli.nativeClosed() { + t.Fatal("self-callback deadline expired before shutdown was initiated") + } + if err := cli.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("outside callback: %v", err) + } +} + +func TestFinalCloseRejectsUnboundedAndCancelledBeforeStop(t *testing.T) { + cli := NewClient(&store.Device{}, nil) + if err := cli.FinalClose(context.Background()); !errors.Is(err, ErrShutdownDeadlineRequired) { + t.Fatalf("unbounded: %v", err) + } + ctx, cancel := context.WithCancel(finalTestContext(t)) + cancel() + if err := cli.FinalClose(ctx); !errors.Is(err, context.Canceled) { + t.Fatalf("cancelled: %v", err) + } + if cli.nativeClosed() { + t.Fatal("refused invocation sealed client") + } + cli.Disconnect() + called := false + cli.AddEventHandler(func(any) { called = true }) + cli.dispatchEvent("ordinary disconnect remains reusable") + if !called { + t.Fatal("Disconnect sealed normal callback admission") + } + if err := cli.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } +} + +type finalCloseTransport struct { + entered chan struct{} + calls atomic.Int32 +} + +func (rt *finalCloseTransport) RoundTrip(req *http.Request) (*http.Response, error) { + rt.calls.Add(1) + close(rt.entered) + <-req.Context().Done() + return nil, req.Context().Err() +} + +func TestFinalCloseCancelsConnectAndPreventsReconnect(t *testing.T) { + id := types.NewJID("synthetic", types.DefaultUserServer) + cli := NewClient(&store.Device{ID: &id}, nil) + transport := &finalCloseTransport{entered: make(chan struct{})} + cli.SetWebsocketHTTPClient(&http.Client{Transport: transport}) + cli.InitialAutoReconnect = true + result := make(chan error, 1) + go func() { result <- cli.ConnectContext(context.Background()) }() + finalTestWait(t, transport.entered) + if err := cli.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("stop dial: %v", err) + } + if err := <-result; err == nil { + t.Fatal("cancelled connect reported success") + } + if err := cli.ConnectContext(finalTestContext(t)); !errors.Is(err, ErrClientClosed) { + t.Fatalf("late connect: %v", err) + } + cli.autoReconnect(context.Background()) + cli.ResetConnection() + if n := transport.calls.Load(); n != 1 { + t.Fatalf("unexpected reconnect: %d dials", n) + } +} + +func TestFinalCloseWaitsAsyncCleanupAndReservedLaunch(t *testing.T) { + cli := NewClient(&store.Device{}, nil) + // A reserved launch is outstanding even before the goroutine is scheduled. + done, ok := cli.enterNative() + if !ok { + t.Fatal("initial admission refused") + } + finalTestBlocked(t, cli) + cleanup := make(chan struct{}) + go func() { defer done(); close(cleanup) }() + finalTestWait(t, cleanup) + if err := cli.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("async cleanup join: %v", err) + } +} + +func TestFinalCloseWaitsRawReceiveCallback(t *testing.T) { + cli := NewClient(&store.Device{}, nil) + entered, release, returned := make(chan struct{}), make(chan struct{}), make(chan struct{}) + defer func() { + select { + case <-release: + default: + close(release) + } + }() + cli.RawNodeHandler = func(context.Context, *waBinary.Node) (*waBinary.Node, bool) { + close(entered) + <-release + return nil, true + } + data, err := waBinary.Marshal(waBinary.Node{Tag: "ack"}) + if err != nil { + t.Fatal(err) + } + go func() { cli.handleFrame(context.Background(), data); close(returned) }() + finalTestWait(t, entered) + cli.Disconnect() + finalTestBlocked(t, cli) + close(release) + finalTestWait(t, returned) + if err := cli.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("receive join: %v", err) + } + // Must not call RawNodeHandler again (it would close entered twice). + cli.handleFrame(context.Background(), data) +} + +type finalStubbornTransport struct{ entered, release chan struct{} } + +func (rt *finalStubbornTransport) RoundTrip(*http.Request) (*http.Response, error) { + close(rt.entered) + <-rt.release + return nil, errors.New("synthetic dial refused") +} +func TestFinalCloseDeadlineWhileConnectOwnsSocketLock(t *testing.T) { + cli := NewClient(&store.Device{}, nil) + rt := &finalStubbornTransport{entered: make(chan struct{}), release: make(chan struct{})} + defer func() { + select { + case <-rt.release: + default: + close(rt.release) + } + }() + cli.SetPreLoginHTTPClient(&http.Client{Transport: rt}) + returned := make(chan error, 1) + go func() { returned <- cli.ConnectContext(context.Background()) }() + finalTestWait(t, rt.entered) + finalTestBlocked(t, cli) + // Teardown is still blocked on socketLock; admission/refusal never waits for it. + if err := cli.ConnectContext(finalTestContext(t)); !errors.Is(err, ErrClientClosed) { + t.Fatalf("late connect blocked or admitted: %v", err) + } + finalTestBlocked(t, cli) + close(rt.release) + if err := <-returned; !errors.Is(err, ErrClientClosed) { + t.Fatalf("sealed in-flight connect: %v", err) + } + if err := cli.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("final dial cleanup: %v", err) + } +} + +type finalRefusingTransport struct{ calls atomic.Int32 } + +func (rt *finalRefusingTransport) RoundTrip(*http.Request) (*http.Response, error) { + rt.calls.Add(1) + return nil, errors.New("synthetic refusal") +} +func TestFinalCloseOrdinaryDisconnectAllowsConnectAgain(t *testing.T) { + cli := NewClient(&store.Device{}, nil) + rt := &finalRefusingTransport{} + cli.SetPreLoginHTTPClient(&http.Client{Transport: rt}) + if err := cli.ConnectContext(finalTestContext(t)); err == nil || errors.Is(err, ErrClientClosed) { + t.Fatalf("first dial: %v", err) + } + cli.Disconnect() + if err := cli.ConnectContext(finalTestContext(t)); err == nil || errors.Is(err, ErrClientClosed) { + t.Fatalf("second dial: %v", err) + } + if rt.calls.Load() != 2 { + t.Fatal("ordinary Disconnect prevented next dial") + } + if err := cli.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("cleanup: %v", err) + } +} diff --git a/keepalive.go b/keepalive.go index b2a8e726c..cc2efa604 100644 --- a/keepalive.go +++ b/keepalive.go @@ -39,20 +39,18 @@ func (cli *Client) keepAliveLoop(ctx, connCtx context.Context) { return } else if !isSuccess { errorCount++ - go cli.dispatchEvent(&events.KeepAliveTimeout{ - ErrorCount: errorCount, - LastSuccess: lastSuccess, - }) + evt := &events.KeepAliveTimeout{ErrorCount: errorCount, LastSuccess: lastSuccess} + cli.startNative(func() { cli.dispatchEvent(evt) }) if cli.EnableAutoReconnect && time.Since(lastSuccess) > KeepAliveMaxFailTime { cli.Log.Debugf("Forcing reconnect due to keepalive failure") cli.Disconnect() cli.resetExpectedDisconnect() - go cli.autoReconnect(ctx) + cli.startNative(func() { cli.autoReconnect(ctx) }) } } else { if errorCount > 0 { errorCount = 0 - go cli.dispatchEvent(&events.KeepAliveRestored{}) + cli.startNative(func() { cli.dispatchEvent(&events.KeepAliveRestored{}) }) } lastSuccess = time.Now() } diff --git a/socket/framesocket.go b/socket/framesocket.go index bcf107b2f..4022b11dc 100644 --- a/socket/framesocket.go +++ b/socket/framesocket.go @@ -20,6 +20,8 @@ import ( ) type FrameSocket struct { + workerLauncher func(func()) bool + parentCtx context.Context cancelCtx context.Context cancel context.CancelFunc @@ -60,6 +62,22 @@ func NewFrameSocket(log waLog.Logger, client *http.Client) *FrameSocket { } } +// SetWorkerLauncher binds socket-owned goroutines to an owning lifetime. Set it +// before Connect; do not change it while the socket is in use. The launcher +// must register work before scheduling and return false after admission closes. +// A nil launcher retains ordinary standalone socket behavior. +func (fs *FrameSocket) SetWorkerLauncher(launcher func(func()) bool) { + fs.workerLauncher = launcher +} + +func (fs *FrameSocket) startWorker(fn func()) bool { + if fs.workerLauncher != nil { + return fs.workerLauncher(fn) + } + go fn() + return true +} + func (fs *FrameSocket) IsConnected() bool { return fs.conn.Load() != nil } @@ -88,7 +106,8 @@ func (fs *FrameSocket) Close(code websocket.StatusCode) { fs.cancel() fs.cancel = nil if fs.OnDisconnect != nil { - go fs.OnDisconnect(fs.parentCtx, code == 0) + callback, parent, remote := fs.OnDisconnect, fs.parentCtx, code == 0 + fs.startWorker(func() { callback(parent, remote) }) } } @@ -114,7 +133,13 @@ func (fs *FrameSocket) Connect(ctx context.Context) error { fs.conn.Store(conn) - go fs.readPump(conn, ctx) + if !fs.startWorker(func() { fs.readPump(conn, ctx) }) { + fs.conn.Store(nil) + fs.closed.Store(true) + fs.cancel() + _ = conn.CloseNow() + return ErrSocketClosed + } return nil } @@ -160,7 +185,10 @@ func (fs *FrameSocket) frameComplete() { fs.partialHeader = nil fs.incomingLength = 0 fs.receivedLength = 0 - fs.Frames <- data + select { + case fs.Frames <- data: + case <-fs.cancelCtx.Done(): + } } func (fs *FrameSocket) processData(msg []byte) { @@ -208,7 +236,7 @@ func (fs *FrameSocket) readPump(conn *websocket.Conn, ctx context.Context) { fs.log.Debugf("Frame websocket read pump starting %p", fs) defer func() { fs.log.Debugf("Frame websocket read pump exiting %p", fs) - go fs.Close(0) + fs.startWorker(func() { fs.Close(0) }) }() for { msgType, data, err := conn.Read(ctx) diff --git a/socket/lifecycle_test.go b/socket/lifecycle_test.go new file mode 100644 index 000000000..c84af607f --- /dev/null +++ b/socket/lifecycle_test.go @@ -0,0 +1,111 @@ +package socket + +import ( + "context" + "errors" + "testing" + "time" + + waLog "github.com/polymorfa/hypermeow/util/log" +) + +type finalTestCipher struct{} + +func (finalTestCipher) NonceSize() int { return 12 } +func (finalTestCipher) Overhead() int { return 0 } +func (finalTestCipher) Seal(dst, nonce, plaintext, extra []byte) []byte { + return append(dst, plaintext...) +} +func (finalTestCipher) Open(dst, nonce, ciphertext, extra []byte) ([]byte, error) { + return append(dst, ciphertext...), nil +} + +func TestFinalSocketAsyncWriteOutlivesCancelledCaller(t *testing.T) { + fs := NewFrameSocket(waLog.Noop, nil) + reserved := make(chan func(), 1) + fs.SetWorkerLauncher(func(fn func()) bool { reserved <- fn; return true }) + ns := &NoiseSocket{fs: fs, writeKey: finalTestCipher{}} + ctx, cancel := context.WithCancel(context.Background()) + result := make(chan error, 1) + go func() { result <- ns.SendFrame(ctx, []byte("synthetic")) }() + var worker func() + select { + case worker = <-reserved: + case <-time.After(time.Second): + t.Fatal("write was not registered") + } + cancel() + if err := <-result; !errors.Is(err, context.Canceled) { + t.Fatalf("caller: %v", err) + } + // The admitted write is still pending. The owner must join this worker even + // though SendFrame has already returned to the cancelled caller. + worker() + fs.SetWorkerLauncher(func(func()) bool { return false }) + if err := ns.SendFrame(context.Background(), nil); !errors.Is(err, ErrSocketClosed) { + t.Fatalf("late write: %v", err) + } +} + +func TestFinalSocketDisconnectUnblocksPendingFrameDelivery(t *testing.T) { + fs := NewFrameSocket(waLog.Noop, nil) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + fs.cancelCtx = ctx + fs.incoming = []byte("synthetic") + done := make(chan struct{}) + go func() { defer close(done); fs.frameComplete() }() + cancel() + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("frame delivery ignored cancellation") + } +} + +func TestFinalSocketReceiveAdmissionIsSealed(t *testing.T) { + fs := NewFrameSocket(waLog.Noop, nil) + fs.SetWorkerLauncher(func(func()) bool { return false }) + ns, err := newNoiseSocket(context.Background(), fs, finalTestCipher{}, finalTestCipher{}, func(context.Context, []byte) { t.Error("late receive callback") }, func(context.Context, *NoiseSocket, bool) { t.Error("late disconnect callback") }) + if ns != nil || !errors.Is(err, ErrSocketClosed) { + t.Fatalf("late receive worker admitted: %v", err) + } +} + +func TestFinalSocketStopDoesNotJoinBlockedReceiveCallback(t *testing.T) { + fs := NewFrameSocket(waLog.Noop, nil) + workerDone := make(chan struct{}) + fs.SetWorkerLauncher(func(fn func()) bool { + go func() { defer close(workerDone); fn() }() + return true + }) + entered, release := make(chan struct{}), make(chan struct{}) + defer func() { + select { + case <-release: + default: + close(release) + } + }() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + ns, err := newNoiseSocket(ctx, fs, finalTestCipher{}, finalTestCipher{}, func(context.Context, []byte) { close(entered); <-release }, func(context.Context, *NoiseSocket, bool) {}) + if err != nil { + t.Fatal(err) + } + fs.Frames <- []byte("synthetic frame") + <-entered + ns.Stop(false, false) + cancel() + select { + case <-workerDone: + t.Fatal("socket stop claimed blocked receive completion") + default: + } + close(release) + select { + case <-workerDone: + case <-time.After(time.Second): + t.Fatal("receive completion was not joined") + } +} diff --git a/socket/noisesocket.go b/socket/noisesocket.go index 8175868ce..d8facd64e 100644 --- a/socket/noisesocket.go +++ b/socket/noisesocket.go @@ -48,7 +48,9 @@ func newNoiseSocket( fs.OnDisconnect = func(ctx context.Context, remote bool) { disconnectHandler(ctx, ns, remote) } - go ns.consumeFrames(ctx, fs.Frames) + if !fs.startWorker(func() { ns.consumeFrames(ctx, fs.Frames) }) { + return nil, ErrSocketClosed + } return ns, nil } @@ -98,9 +100,11 @@ func (ns *NoiseSocket) SendFrame(ctx context.Context, plaintext []byte) error { ciphertext := ns.writeKey.Seal(nil, generateIV(ns.writeCounter), plaintext, nil) ns.writeCounter++ doneChan := make(chan error, 1) - go func() { + if !ns.fs.startWorker(func() { doneChan <- ns.fs.SendFrame(ciphertext) - }() + }) { + return ErrSocketClosed + } select { case <-ctx.Done(): return ctx.Err() From e7d513e934100d58a95b4469b124395a25f32987 Mon Sep 17 00:00:00 2001 From: Rajeh Taher Date: Thu, 22 Oct 2026 06:13:25 +0300 Subject: [PATCH 02/10] Fix reconnect admission race and forwarding context ownership --- CLIENT_LIFETIME.md | 26 ++++ client.go | 15 +- client_lifecycle.go | 114 ++++++++++++-- client_lifecycle_context_test.go | 258 +++++++++++++++++++++++++++++++ keepalive.go | 2 +- 5 files changed, 396 insertions(+), 19 deletions(-) create mode 100644 client_lifecycle_context_test.go diff --git a/CLIENT_LIFETIME.md b/CLIENT_LIFETIME.md index f3f887bec..2783b06fa 100644 --- a/CLIENT_LIFETIME.md +++ b/CLIENT_LIFETIME.md @@ -42,6 +42,32 @@ remaining stuck sending to an abandoned consumer. Socket ciphertext, counters, keys, handshake and wire formats are unchanged. Standalone FrameSocket users keep the default launcher unless they explicitly install their own before Connect. +## Forwarding context ownership + +Each connect attempt owns a forwarding context and releases that ownership on +return, including failed, deleted-store and already-connected paths. If initial +background reconnect is requested, ConnectContext returns nil only when that +reconnect task was actually admitted. Refused admission returns ErrClientClosed. +The admitted task retains the context before scheduling and releases it on exit. +Its internal attempts reuse the same forwarding scope instead of adding one +watcher per retry. Once no owner remains, cleanup unregisters the lifetime +AfterFunc and cancels the derived context. + +A successful handshake transfers a context reference to the client lifetime +before ConnectContext can return. These established contexts remain owned until +final close, including across ordinary Disconnect. This is conservative: the +unbound producers below may still inherit an established event context after its +socket workers exit. Their exact completion is not known, so early cancellation +would change ordinary behavior. Reconnect attempts with that inherited context +reuse the same scope. Explicit new successful connections with different caller +contexts can retain separate scopes until final close. This is not a claim of +complete context/producer joins or bounded successful-connection inventory. + +Final close cancels the lifetime outside state locks, waits for the tracked +subset, and releases the established-context references. No successful return or +stop receipt follows from that cleanup. Tests of the context-transfer helper do +not establish a live handshake/socket acceptance result. + ## Still unbound The following existing client-owned producers retain their prior behavior and diff --git a/client.go b/client.go index 74298499a..8895e986f 100644 --- a/client.go +++ b/client.go @@ -591,7 +591,8 @@ func (cli *Client) ConnectContext(ctx context.Context) error { return ErrClientClosed } defer done() - ctx = cli.nativeContext(ctx) + ctx, releaseContext := cli.nativeContext(ctx) + defer releaseContext() cli.socketLock.Lock() defer cli.socketLock.Unlock() @@ -602,7 +603,9 @@ func (cli *Client) ConnectContext(ctx context.Context) error { if isRetryableConnectError(err) && cli.InitialAutoReconnect && cli.EnableAutoReconnect { cli.Log.Errorf("Initial connection failed but reconnecting in background (%v)", err) cli.startNative(func() { cli.dispatchEvent(&events.Disconnected{}) }) - cli.startNative(func() { cli.autoReconnect(ctx) }) + if !cli.startNativeContext(ctx, func() { cli.autoReconnect(ctx) }) { + return ErrClientClosed + } return nil } return err @@ -614,7 +617,8 @@ func (cli *Client) connect(ctx context.Context) error { return ErrClientClosed } defer done() - ctx = cli.nativeContext(ctx) + ctx, releaseContext := cli.nativeContext(ctx) + defer releaseContext() cli.socketLock.Lock() defer cli.socketLock.Unlock() @@ -673,6 +677,9 @@ func (cli *Client) unlockedConnect(ctx context.Context) error { fs.Close(0) return fmt.Errorf("noise handshake failed: %w", err) } + if !cli.pinNativeContext(ctx) { + return ErrClientClosed + } cli.startNative(func() { cli.keepAliveLoop(ctx, fs.Context()) }) cli.startNative(func() { cli.handlerQueueLoop(ctx, fs.Context()) }) return nil @@ -693,7 +700,7 @@ func (cli *Client) onDisconnect(ctx context.Context, ns *socket.NoiseSocket, rem if !cli.isExpectedDisconnect() && (cli.forceAutoReconnect.Swap(false) || remote) { cli.Log.Debugf("Emitting Disconnected event") cli.startNative(func() { cli.dispatchEvent(&events.Disconnected{}) }) - cli.startNative(func() { cli.autoReconnect(ctx) }) + cli.startNativeContext(ctx, func() { cli.autoReconnect(ctx) }) } else if remote { cli.Log.Debugf("OnDisconnect() called, but it was expected, so not emitting event") } else { diff --git a/client_lifecycle.go b/client_lifecycle.go index 1ccdcf19b..e57616cd5 100644 --- a/client_lifecycle.go +++ b/client_lifecycle.go @@ -20,14 +20,15 @@ var ( ) type clientLifetime struct { - mu sync.Mutex - ctx context.Context - cancel context.CancelFunc - sealed bool - active int - joined chan struct{} - stopped chan struct{} - sockets map[*socket.FrameSocket]int + mu sync.Mutex + ctx context.Context + cancel context.CancelFunc + sealed bool + active int + joined chan struct{} + stopped chan struct{} + sockets map[*socket.FrameSocket]int + contexts map[*nativeClientContext]struct{} } func (l *clientLifetime) initLocked() { @@ -35,6 +36,7 @@ func (l *clientLifetime) initLocked() { l.ctx, l.cancel = context.WithCancel(context.Background()) l.joined = make(chan struct{}) l.sockets = make(map[*socket.FrameSocket]int) + l.contexts = make(map[*nativeClientContext]struct{}) } } @@ -113,19 +115,96 @@ func (cli *Client) bindSocket(fs *socket.FrameSocket) func() { return release } -// nativeContext retains the caller's cancellation and adds final-close -// cancellation. Its cancellation callback only cancels a context; it performs -// no user callback, socket operation or protocol work. -func (cli *Client) nativeContext(parent context.Context) context.Context { +// nativeClientContext has explicit owners: a connect attempt, an admitted +// reconnect task, and (after a successful handshake) this client lifetime. +// Reconnect attempts reuse their inherited owner instead of growing a chain +// of cancellation watchers. Release never holds a client state lock. +type nativeClientContext struct { + context.Context + lifetime *clientLifetime + mu sync.Mutex + refs int + cleanup context.CancelFunc +} + +func (ctx *nativeClientContext) retain() bool { + ctx.mu.Lock() + defer ctx.mu.Unlock() + if ctx.refs == 0 { + return false + } + ctx.refs++ + return true +} + +func (ctx *nativeClientContext) release() { + ctx.mu.Lock() + ctx.refs-- + var cleanup context.CancelFunc + if ctx.refs == 0 { + cleanup = ctx.cleanup + ctx.cleanup = nil + } + ctx.mu.Unlock() + if cleanup != nil { + cleanup() + } +} + +// Every attempt releases its owner on return. An admitted reconnect retains it +// until that task returns. An established socket pins it until final close: +// still-unbound async producers may inherit the event context after Disconnect, +// so cancelling it at ordinary socket/loop exit would change their behavior. +func (cli *Client) nativeContext(parent context.Context) (context.Context, context.CancelFunc) { l := &cli.lifetime + if inherited, ok := parent.(*nativeClientContext); ok && inherited.lifetime == l && inherited.retain() { + return inherited, inherited.release + } l.mu.Lock() l.initLocked() lifetime := l.ctx l.mu.Unlock() ctx, cancel := context.WithCancel(parent) stop := context.AfterFunc(lifetime, cancel) - context.AfterFunc(ctx, func() { stop(); cancel() }) - return ctx + owned := &nativeClientContext{Context: ctx, lifetime: l, refs: 1, cleanup: func() { stop(); cancel() }} + return owned, owned.release +} + +// Keep ownership before admission/scheduling, and release even if admission is +// refused. This does not turn admission into a successful connection or join. +func (cli *Client) startNativeContext(ctx context.Context, fn func()) bool { + release := func() {} + if owned, ok := ctx.(*nativeClientContext); ok && owned.lifetime == &cli.lifetime { + if !owned.retain() { + return false + } + release = owned.release + } + if !cli.startNative(func() { defer release(); fn() }) { + release() + return false + } + return true +} + +func (cli *Client) pinNativeContext(ctx context.Context) bool { + owned, ok := ctx.(*nativeClientContext) + if !ok || owned.lifetime != &cli.lifetime { + return true + } // Caller-owned internal contexts. + l := &cli.lifetime + l.mu.Lock() + defer l.mu.Unlock() + if l.sealed { + return false + } + if _, exists := l.contexts[owned]; !exists { + if !owned.retain() { + return false + } + l.contexts[owned] = struct{}{} + } + return true } // FinalClose permanently seals this exact Client pointer. Unlike Disconnect, @@ -194,5 +273,12 @@ func (cli *Client) finalDisconnect() { fs.Close(0) } <-l.joined + l.mu.Lock() + contexts := l.contexts + l.contexts = nil + l.mu.Unlock() + for owned := range contexts { + owned.release() + } close(l.stopped) } diff --git a/client_lifecycle_context_test.go b/client_lifecycle_context_test.go new file mode 100644 index 000000000..768e070a9 --- /dev/null +++ b/client_lifecycle_context_test.go @@ -0,0 +1,258 @@ +package whatsmeow + +import ( + "context" + "errors" + "github.com/polymorfa/hypermeow/store" + "github.com/polymorfa/hypermeow/types" + waLog "github.com/polymorfa/hypermeow/util/log" + "net/http" + "reflect" + "strings" + "sync/atomic" + "testing" + "time" +) + +type finalContextRefusal struct{} + +func (finalContextRefusal) RoundTrip(*http.Request) (*http.Response, error) { + return nil, errors.New("synthetic refusal") +} + +type finalContextBarrierLog struct { + waLog.Logger + entered, release chan struct{} +} + +func (l *finalContextBarrierLog) Errorf(msg string, args ...any) { + if strings.HasPrefix(msg, "Initial connection failed but reconnecting") { + close(l.entered) + <-l.release + } +} +func TestFinalCloseInitialReconnectSealRace(t *testing.T) { + l := &finalContextBarrierLog{Logger: waLog.Noop, entered: make(chan struct{}), release: make(chan struct{})} + c := NewClient(&store.Device{}, l) + c.InitialAutoReconnect = true + c.SetPreLoginHTTPClient(&http.Client{Transport: finalContextRefusal{}}) + result := make(chan error, 1) + go func() { result <- c.ConnectContext(context.Background()) }() + select { + case <-l.entered: + case <-time.After(time.Second): + t.Fatal("no reconnect log") + } + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + stopped := make(chan error, 1) + go func() { stopped <- c.FinalClose(ctx) }() + select { + case <-c.nativeDone(): + case <-time.After(time.Second): + t.Fatal("not sealed") + } + close(l.release) + err := <-result + closeErr := <-stopped + if !errors.Is(closeErr, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", closeErr) + } + if !errors.Is(err, ErrClientClosed) { + t.Fatalf("sealed reconnect admission must return ErrClientClosed: %v", err) + } +} +func TestFinalCloseFailedAttemptsReleaseContextWatchers(t *testing.T) { + c := NewClient(&store.Device{}, nil) + c.SetPreLoginHTTPClient(&http.Client{Transport: finalContextRefusal{}}) + for i := 0; i < 100; i++ { + if c.ConnectContext(context.Background()) == nil { + t.Fatal("unexpected dial success") + } + } + c.lifetime.mu.Lock() + retained := reflect.ValueOf(c.lifetime.ctx).Elem().FieldByName("children").Len() + c.lifetime.mu.Unlock() + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + if err := c.FinalClose(ctx); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("cleanup: %v", err) + } + if retained != 0 { + t.Fatalf("100 completed failed dials retained %d lifetime cancellation watchers before FinalClose", retained) + } +} + +// Inspect only quiescent forwarding registrations on the pinned Go toolchain. +// This is the R1 independent observation, not a replacement implementation. +func finalContextWatcherCount(c *Client) int { + c.lifetime.mu.Lock() + defer c.lifetime.mu.Unlock() + return reflect.ValueOf(c.lifetime.ctx).Elem().FieldByName("children").Len() +} + +func TestFinalCloseInternalFailedAndEarlyAttemptsReleaseContexts(t *testing.T) { + c := NewClient(&store.Device{}, nil) + c.SetPreLoginHTTPClient(&http.Client{Transport: finalContextRefusal{}}) + for i := 0; i < 100; i++ { + if err := c.connect(context.Background()); err == nil { + t.Fatal("unexpected internal dial success") + } + } + // Store-deleted is an early return before socket creation. The same deferred + // release also covers ErrAlreadyConnected; no live socket is fabricated here. + c.Store.Deleted = true + for i := 0; i < 100; i++ { + if err := c.ConnectContext(context.Background()); !errors.Is(err, store.ErrDeviceDeleted) { + t.Fatalf("early return: %v", err) + } + } + if count := finalContextWatcherCount(c); count != 0 { + t.Fatalf("completed internal/early attempts retained %d watchers", count) + } + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } +} + +// This uses the same ownership-transfer seam as a completed handshake, without +// claiming any handshake, live socket or provider acceptance. +func TestFinalCloseEstablishedContextSurvivesAttemptAndDisconnect(t *testing.T) { + c := NewClient(&store.Device{}, nil) + ctx, releaseAttempt := c.nativeContext(context.Background()) + if !c.pinNativeContext(ctx) { + t.Fatal("established ownership refused") + } + releaseAttempt() + if ctx.Err() != nil { + t.Fatalf("attempt return cancelled established context: %v", ctx.Err()) + } + c.Disconnect() + if ctx.Err() != nil { + t.Fatal("ordinary Disconnect cancelled inherited unbound event work") + } + if count := finalContextWatcherCount(c); count != 1 { + t.Fatalf("established lifetime must own exactly one watcher, got %d", count) + } + // Every inherited reconnect attempt reuses the scope, without a nested + // forwarding watcher or a cancellation of the established parent. + for i := 0; i < 100; i++ { + attempt, release := c.nativeContext(ctx) + if attempt != ctx { + t.Fatal("inherited reconnect allocated a nested context") + } + release() + } + if ctx.Err() != nil || finalContextWatcherCount(c) != 1 { + t.Fatal("reconnect attempt changed established context lifetime") + } + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } + if !errors.Is(ctx.Err(), context.Canceled) { + t.Fatalf("final close did not cancel established context: %v", ctx.Err()) + } + if count := finalContextWatcherCount(c); count != 0 { + t.Fatalf("final close retained %d watchers", count) + } +} + +type finalContextReconnectTransport struct { + calls atomic.Int32 + entered chan context.Context +} + +func (rt *finalContextReconnectTransport) RoundTrip(req *http.Request) (*http.Response, error) { + if rt.calls.Add(1) == 1 { + return nil, errors.New("synthetic initial refusal") + } + rt.entered <- req.Context() + <-req.Context().Done() + return nil, req.Context().Err() +} + +func TestFinalCloseAdmittedReconnectOwnsContextAfterInitialReturn(t *testing.T) { + id := types.NewJID("synthetic", types.DefaultUserServer) + c := NewClient(&store.Device{ID: &id}, nil) + c.InitialAutoReconnect = true + rt := &finalContextReconnectTransport{entered: make(chan context.Context, 1)} + c.SetWebsocketHTTPClient(&http.Client{Transport: rt}) + if err := c.ConnectContext(context.Background()); err != nil { + t.Fatalf("initial reconnect admission: %v", err) + } + var reconnectCtx context.Context + select { + case reconnectCtx = <-rt.entered: + case <-time.After(2 * time.Second): + t.Fatal("admitted reconnect lost its context before dialing") + } + if err := reconnectCtx.Err(); err != nil { + t.Fatalf("attempt return cancelled admitted reconnect: %v", err) + } + if count := finalContextWatcherCount(c); count != 1 { + t.Fatalf("reconnect must reuse one forwarding scope, got %d watchers", count) + } + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } + if rt.calls.Load() != 2 { + t.Fatalf("unexpected late/repeated dial count: %d", rt.calls.Load()) + } + if count := finalContextWatcherCount(c); count != 0 { + t.Fatalf("cancelled reconnect retained %d watchers", count) + } +} + +type finalContextRecordingRefusal struct { + first chan context.Context + calls atomic.Int32 +} + +func (rt *finalContextRecordingRefusal) RoundTrip(req *http.Request) (*http.Response, error) { + if rt.calls.Add(1) == 1 { + rt.first <- req.Context() + } + return nil, errors.New("synthetic refusal") +} +func TestFinalCloseStoppedReconnectReleasesContextWithoutFinalClose(t *testing.T) { + id := types.NewJID("synthetic", types.DefaultUserServer) + c := NewClient(&store.Device{ID: &id}, nil) + c.InitialAutoReconnect = true + c.AutoReconnectHook = func(error) bool { return false } + rt := &finalContextRecordingRefusal{first: make(chan context.Context, 1)} + c.SetWebsocketHTTPClient(&http.Client{Transport: rt}) + if err := c.ConnectContext(context.Background()); err != nil { + t.Fatalf("initial admission: %v", err) + } + inherited := <-rt.first + select { + case <-inherited.Done(): + case <-time.After(2 * time.Second): + t.Fatal("ended reconnect retained its forwarding context") + } + if count := finalContextWatcherCount(c); count != 0 { + t.Fatalf("ended reconnect retained %d watchers before FinalClose", count) + } + if rt.calls.Load() != 2 { + t.Fatalf("unexpected dial count: %d", rt.calls.Load()) + } + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } +} + +func TestFinalCloseRefusedContextLaunchReleasesOnlyItsReservation(t *testing.T) { + c := NewClient(&store.Device{}, nil) + ctx, release := c.nativeContext(context.Background()) + // Model the attempt's outstanding owner while a final seal wins admission. + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } + if c.startNativeContext(ctx, func() { t.Error("late reconnect ran") }) { + t.Fatal("late context worker admitted") + } + release() + if count := finalContextWatcherCount(c); count != 0 { + t.Fatalf("refused launch retained %d watchers", count) + } +} diff --git a/keepalive.go b/keepalive.go index cc2efa604..ae0c7a0d4 100644 --- a/keepalive.go +++ b/keepalive.go @@ -45,7 +45,7 @@ func (cli *Client) keepAliveLoop(ctx, connCtx context.Context) { cli.Log.Debugf("Forcing reconnect due to keepalive failure") cli.Disconnect() cli.resetExpectedDisconnect() - cli.startNative(func() { cli.autoReconnect(ctx) }) + cli.startNativeContext(ctx, func() { cli.autoReconnect(ctx) }) } } else { if errorCount > 0 { From 3b6090ef96b81a677b9c169e7f47e99a87810888 Mon Sep 17 00:00:00 2001 From: Rajeh Taher Date: Thu, 22 Oct 2026 07:10:11 +0300 Subject: [PATCH 03/10] Join connection event tasks and QR channel cleanup lifetimes --- CLIENT_LIFETIME.md | 43 +++- client_lifecycle.go | 16 ++ connection_lifecycle_test.go | 130 +++++++++++ connectionevents.go | 28 +-- qr_lifecycle_test.go | 403 +++++++++++++++++++++++++++++++++++ qrchan.go | 261 +++++++++++++++++------ 6 files changed, 795 insertions(+), 86 deletions(-) create mode 100644 connection_lifecycle_test.go create mode 100644 qr_lifecycle_test.go diff --git a/CLIENT_LIFETIME.md b/CLIENT_LIFETIME.md index 2783b06fa..49dc61a6b 100644 --- a/CLIENT_LIFETIME.md +++ b/CLIENT_LIFETIME.md @@ -31,6 +31,14 @@ the deadline ends its waiter and does not manufacture completion. asynchronous socket close, and asynchronous writes. Client installs the socket launcher before dialing. The launcher records owned completion before scheduling. A write remains tracked after SendFrame returns on context cancellation. +- Connection-event async dispatch wrappers, the 515 disconnect/reconnect task, + and the connect-success task. Context-aware tasks acquire an owned forwarding + context before scheduling; their existing store/prekey/IQ bodies are unchanged. +- GetQRChannel registration and its one cleanup worker, each admitted QR emitter, + output sends, terminal closure and exact handler removal. The channel lifetime + starts before Connect, including when no QR event ever arrives. Caller/final + cancellation ends that lifetime. Ordinary Disconnect stops an emitter as before + and leaves the channel available for subsequent events. - One teardown task, whose completion is recorded before launch. Context forwarding uses context.AfterFunc only to cancel contexts, with no user/protocol callbacks; those small cancellation callbacks are not a joined product-worker claim. @@ -68,6 +76,37 @@ subset, and releases the established-context references. No successful return or stop receipt follows from that cleanup. Tests of the context-transfer helper do not establish a live handshake/socket acceptance result. +## QR channel closure and boundaries + +One reserved cleanup worker owns each output close and handler removal. Sends +register under the QR-local mutex, then wait without it. Closure seals new sends +and waits for existing sends outside locks before closing output. Terminal event +callbacks are released before cleanup acquires the handler-list write lock, so +removal cannot deadlock on the callback's own read lock. FinalClose joins the +cleanup and emitter tasks even if removal is blocked by another callback. +QR-local closure and emitter admission also share one lock. Timers retain the +existing 20-second/60-second values and are stopped on every interrupted wait. + +Caller cancellation or final close can close output without a terminal pairing +status. Output backpressure keeps its prior nonblocking code-emission shutdown +behavior. A caller must not treat output close as a successful pairing, a joined +whole-client lifetime or a stop receipt. The cleanup worker can still be removing +its handler after output closes; FinalClose tracks that completion separately. + +Expiry/caller cancellation may disconnect only the exact socket pointer observed +by the QR emitter if it is still current. No observed socket means no disconnect; +a retired pointer cannot stop a replacement socket on the same Client. This is +local lifetime protection, not authenticated event-origin or placement custody. +Synthetic injected QR events and externally started operations remain the caller's +responsibility. No pairing/protocol/Signal/key/store body or payload changed. +The existing automatic passkey-confirmation method is invoked synchronously by +its admitted QR callback, not reclassified as an accepted Signal producer. + +Notifications/AB-props and privacy pruning were deliberately not folded into +this group. Connection-success's pruning call still reaches an unbound task as +listed below. External RefreshCAT implementations, store/transport internals and +user-spawned callbacks do not gain owned completion proof from these wrappers. + ## Still unbound The following existing client-owned producers retain their prior behavior and @@ -79,14 +118,12 @@ owning-library work before any successful stop result is possible. | Source at base 29a42459 | Unbound lifetime | | --- | --- | -| connectionevents.go:33,187 | Stream cleanup, connect-success/prekey/upload/session work and async event-dispatch launch wrappers. Event callback invocation itself is sealed by dispatchEvent, but the wrapper goroutine is unbound. | | notification.go:57,522-566,603 | Privacy token, encryption/app-state/account-sync, code/passkey pairing, and server AB-props sync tasks. | | message.go:82-89,373-374,469-470,520,549,770,887,1005-1037 | Name updates, retry/ack work, event-buffer cleanup, event-dispatch wrapper, history download/storage/notification loops, key-share and peer-data work. | | appstate.go:162,645,659 | Missing-key request and async event batches. | | pair.go:138-147 | Pair completion, PrePairCallback and unified-session child task. | | receipt.go:27,36,109,128; retry.go:469,541 | Async ack/retry handlers and delayed phone rerequests. Cancellation of delayed requests is not their join. | -| presence.go:71; send.go:903; tctoken.go:150 | Unified-session, privacy-token persistence and expired-token pruning. | -| qrchan.go:80,102,119,135,195 | QR emission and async handler removals. | +| presence.go:71; send.go:903; tctoken.go:150 | Unified-session, privacy-token persistence and expired-token pruning. The prune called synchronously by connect-success still launches this separate unbound task. | | handshake.go:98-103 | GetClientPayload callback admission is not separately sealed; an admitted connection can reach it. The enclosing connect is counted, which is not full callback-admission coverage. | Caller-invoked public API methods, DangerousInternals, external HTTP transports, diff --git a/client_lifecycle.go b/client_lifecycle.go index e57616cd5..970e887e0 100644 --- a/client_lifecycle.go +++ b/client_lifecycle.go @@ -70,6 +70,22 @@ func (cli *Client) startNative(fn func()) bool { return true } +// startConnectionTask owns a connection-event task and its forwarding context. +// Registration precedes scheduling even when final close wins before execution. +func (cli *Client) startConnectionTask(parent context.Context, fn func(context.Context)) bool { + done, ok := cli.enterNative() + if !ok { + return false + } + ctx, release := cli.nativeContext(parent) + go func() { defer done(); defer release(); fn(ctx) }() + return true +} + +func (cli *Client) dispatchEventAsync(evt any) bool { + return cli.startNative(func() { cli.dispatchEvent(evt) }) +} + func (cli *Client) nativeDone() <-chan struct{} { l := &cli.lifetime l.mu.Lock() diff --git a/connection_lifecycle_test.go b/connection_lifecycle_test.go new file mode 100644 index 000000000..6477c7a18 --- /dev/null +++ b/connection_lifecycle_test.go @@ -0,0 +1,130 @@ +package whatsmeow + +import ( + "context" + "errors" + "net/http" + "strings" + "sync" + "testing" + "time" + + waBinary "github.com/polymorfa/hypermeow/binary" + "github.com/polymorfa/hypermeow/store" + "github.com/polymorfa/hypermeow/types/events" + waLog "github.com/polymorfa/hypermeow/util/log" +) + +type finalConnectionBarrierLog struct { + waLog.Logger + prefix string + entered, release chan struct{} + once sync.Once +} + +func (l *finalConnectionBarrierLog) Sub(string) waLog.Logger { return l } +func (l *finalConnectionBarrierLog) Infof(msg string, args ...any) { + if strings.HasPrefix(msg, l.prefix) { + l.once.Do(func() { close(l.entered); <-l.release }) + } +} + +func TestFinalConnectionEventLaunchLosesToSeal(t *testing.T) { + log := &finalConnectionBarrierLog{Logger: waLog.Noop, prefix: "Got replaced stream error", entered: make(chan struct{}), release: make(chan struct{})} + c := NewClient(&store.Device{}, log) + called := false + c.AddEventHandler(func(any) { called = true }) + c.startNative(func() { + c.handleStreamError(context.Background(), &waBinary.Node{Tag: "stream:error", Attrs: waBinary.Attrs{"code": "409"}, Content: []waBinary.Node{{Tag: "conflict", Attrs: waBinary.Attrs{"type": "replaced"}}}}) + }) + finalTestWait(t, log.entered) + finalTestBlocked(t, c) + close(log.release) + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } + if called { + t.Fatal("late connection-event callback admitted") + } +} + +type finalConnectionPrekeys struct { + store.NoopStore + entered chan context.Context + release chan struct{} +} + +func (s *finalConnectionPrekeys) UploadedPreKeyCount(ctx context.Context) (int, error) { + s.entered <- ctx + <-s.release + return 0, errors.New("synthetic prekey inventory refusal") +} + +type finalConnectionContextKey struct{} + +func TestFinalConnectionSuccessJoinsBlockedOwnedTask(t *testing.T) { + blocking := &finalConnectionPrekeys{entered: make(chan context.Context, 1), release: make(chan struct{})} + c := NewClient(&store.Device{PreKeys: blocking}, nil) + c.paired.Store(true) + c.lastTCTokenDBPrune = time.Now() // Do not launch the separately unbound prune. + parent := context.WithValue(context.Background(), finalConnectionContextKey{}, "preserved-value") + c.handleConnectSuccess(parent, &waBinary.Node{Tag: "success", Attrs: waBinary.Attrs{"t": "1"}}) + var inherited context.Context + select { + case inherited = <-blocking.entered: + case <-time.After(2 * time.Second): + t.Fatal("post-connect task was not launched") + } + if inherited.Value(finalConnectionContextKey{}) != "preserved-value" { + t.Fatal("event context value lost") + } + finalTestBlocked(t, c) + if !errors.Is(inherited.Err(), context.Canceled) { + t.Fatal("post-connect task did not receive final cancellation") + } + close(blocking.release) + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("post-connect join: %v", err) + } + if finalContextWatcherCount(c) != 0 { + t.Fatal("post-connect forwarding ownership leaked") + } +} + +func TestFinalConnectionAsyncEventPreservesOrdinaryDispatch(t *testing.T) { + c := NewClient(&store.Device{}, nil) + received := make(chan any, 1) + c.AddEventHandler(func(evt any) { received <- evt }) + c.Disconnect() + c.handleIB(context.Background(), &waBinary.Node{Tag: "ib", Content: []waBinary.Node{{Tag: "downgrade_webclient"}}}) + select { + case evt := <-received: + if _, ok := evt.(*events.QRScannedWithoutMultidevice); !ok { + t.Fatalf("event type changed: %T", evt) + } + case <-time.After(2 * time.Second): + t.Fatal("ordinary async event was sealed") + } + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } +} + +func TestFinalConnectionCleanupReconnectLaunchLosesToSeal(t *testing.T) { + log := &finalConnectionBarrierLog{Logger: waLog.Noop, prefix: "Got 515 code, reconnecting", entered: make(chan struct{}), release: make(chan struct{})} + c := NewClient(&store.Device{}, log) + rt := &finalRefusingTransport{} + c.SetPreLoginHTTPClient(&http.Client{Transport: rt}) + c.startNative(func() { + c.handleStreamError(context.Background(), &waBinary.Node{Tag: "stream:error", Attrs: waBinary.Attrs{"code": "515"}}) + }) + finalTestWait(t, log.entered) + finalTestBlocked(t, c) + close(log.release) + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } + if rt.calls.Load() != 0 { + t.Fatal("sealed 515 cleanup admitted reconnect") + } +} diff --git a/connectionevents.go b/connectionevents.go index 3e52853d3..e9beb75bb 100644 --- a/connectionevents.go +++ b/connectionevents.go @@ -30,17 +30,17 @@ func (cli *Client) handleStreamError(ctx context.Context, node *waBinary.Node) { return } cli.Log.Infof("Got 515 code, reconnecting...") - go func() { + cli.startConnectionTask(ctx, func(ctx context.Context) { cli.Disconnect() err := cli.connect(ctx) if err != nil { cli.Log.Errorf("Failed to reconnect after 515 code: %v", err) } - }() + }) case code == "401" && conflictType == "device_removed": cli.expectDisconnect() cli.Log.Infof("Got device removed stream error, sending LoggedOut event and deleting session") - go cli.dispatchEvent(&events.LoggedOut{OnConnect: false, Reason: events.ConnectFailureLoggedOut}) + cli.dispatchEventAsync(&events.LoggedOut{OnConnect: false, Reason: events.ConnectFailureLoggedOut}) err := cli.Store.Delete(ctx) if err != nil { cli.Log.Warnf("Failed to delete store after device_removed error: %v", err) @@ -48,7 +48,7 @@ func (cli *Client) handleStreamError(ctx context.Context, node *waBinary.Node) { case conflictType == "replaced": cli.expectDisconnect() cli.Log.Infof("Got replaced stream error, sending StreamReplaced event") - go cli.dispatchEvent(&events.StreamReplaced{}) + cli.dispatchEventAsync(&events.StreamReplaced{}) case code == "503": // This seems to happen when the server wants to restart or something. // The disconnection will be emitted as an events.Disconnected and then the auto-reconnect will do its thing. @@ -61,11 +61,11 @@ func (cli *Client) handleStreamError(ctx context.Context, node *waBinary.Node) { if err != nil { cli.Log.Errorf("Failed to refresh CAT: %v", err) cli.expectDisconnect() - go cli.dispatchEvent(&events.CATRefreshError{Error: err}) + cli.dispatchEventAsync(&events.CATRefreshError{Error: err}) } default: cli.Log.Errorf("Unknown stream error: %s", node) - go cli.dispatchEvent(&events.StreamError{Code: code, Raw: node}) + cli.dispatchEventAsync(&events.StreamError{Code: code, Raw: node}) } } @@ -75,7 +75,7 @@ func (cli *Client) handleIB(ctx context.Context, node *waBinary.Node) { ag := child.AttrGetter() switch child.Tag { case "downgrade_webclient": - go cli.dispatchEvent(&events.QRScannedWithoutMultidevice{}) + cli.dispatchEventAsync(&events.QRScannedWithoutMultidevice{}) case "offline_preview": cli.dispatchEvent(&events.OfflineSyncPreview{ Total: ag.Int("count"), @@ -125,33 +125,33 @@ func (cli *Client) handleConnectFailure(ctx context.Context, node *waBinary.Node } if reason.IsLoggedOut() { cli.Log.Infof("Got %s connect failure, sending LoggedOut event and deleting session", reason) - go cli.dispatchEvent(&events.LoggedOut{OnConnect: true, Reason: reason}) + cli.dispatchEventAsync(&events.LoggedOut{OnConnect: true, Reason: reason}) err := cli.Store.Delete(ctx) if err != nil { cli.Log.Warnf("Failed to delete store after %d failure: %v", int(reason), err) } } else if reason == events.ConnectFailureTempBanned { cli.Log.Warnf("Temporary ban connect failure: %s", node) - go cli.dispatchEvent(&events.TemporaryBan{ + cli.dispatchEventAsync(&events.TemporaryBan{ Code: events.TempBanReason(ag.Int("code")), Expire: time.Duration(ag.Int("expire")) * time.Second, }) } else if reason == events.ConnectFailureClientOutdated { cli.Log.Errorf("Client outdated (405) connect failure (client version: %s)", store.GetWAVersion().String()) - go cli.dispatchEvent(&events.ClientOutdated{}) + cli.dispatchEventAsync(&events.ClientOutdated{}) } else if reason == events.ConnectFailureCATInvalid || reason == events.ConnectFailureCATExpired { cli.Log.Infof("Got %d/%s connect failure, refreshing CAT before reconnecting...", int(reason), message) err := cli.RefreshCAT(ctx) if err != nil { cli.Log.Errorf("Failed to refresh CAT: %v", err) cli.expectDisconnect() - go cli.dispatchEvent(&events.CATRefreshError{Error: err}) + cli.dispatchEventAsync(&events.CATRefreshError{Error: err}) } } else if willAutoReconnect { cli.Log.Warnf("Got %d/%s connect failure, assuming automatic reconnect will handle it", int(reason), message) } else { cli.Log.Warnf("Unknown connect failure: %s", node) - go cli.dispatchEvent(&events.ConnectFailure{Reason: reason, Message: message, Raw: node}) + cli.dispatchEventAsync(&events.ConnectFailure{Reason: reason, Message: message, Raw: node}) } } @@ -184,7 +184,7 @@ func (cli *Client) handleConnectSuccess(ctx context.Context, node *waBinary.Node cli.StoreLIDPNMapping(ctx, cli.Store.GetLID(), cli.Store.GetJID()) } cli.deleteExpiredPrivacyTokens() - go func() { + cli.startConnectionTask(ctx, func(ctx context.Context) { if dbCount, err := cli.Store.PreKeys.UploadedPreKeyCount(ctx); err != nil { cli.Log.Errorf("Failed to get number of prekeys in database: %v", err) } else if serverCount, err := cli.getServerPreKeyCount(ctx); err != nil { @@ -203,7 +203,7 @@ func (cli *Client) handleConnectSuccess(ctx context.Context, node *waBinary.Node } cli.dispatchEvent(&events.Connected{}) cli.closeSocketWaitChan() - }() + }) } // SetPassive tells the WhatsApp server whether this device is passive or not. diff --git a/qr_lifecycle_test.go b/qr_lifecycle_test.go new file mode 100644 index 000000000..459620c81 --- /dev/null +++ b/qr_lifecycle_test.go @@ -0,0 +1,403 @@ +package whatsmeow + +import ( + "context" + "errors" + "github.com/polymorfa/hypermeow/socket" + "github.com/polymorfa/hypermeow/types/events" + waLog "github.com/polymorfa/hypermeow/util/log" + "strings" + "sync" + "testing" + "time" + + "github.com/polymorfa/hypermeow/store" +) + +func TestFinalQRIdleCancellationClosesAndCleansHandler(t *testing.T) { + cli := NewClient(&store.Device{}, nil) + ctx, cancel := context.WithCancel(context.Background()) + qr, err := cli.GetQRChannel(ctx) + if err != nil { + t.Fatal(err) + } + cancel() + select { + case _, open := <-qr: + if open { + t.Fatal("cancelled idle channel emitted unexpected item") + } + case <-time.After(time.Second): + t.Fatal("idle GetQRChannel did not close on cancellation") + } + if err := cli.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } + cli.eventHandlersLock.RLock() + count := len(cli.eventHandlers) + cli.eventHandlersLock.RUnlock() + if count != 0 { + t.Fatalf("QR handler was not removed: %d", count) + } +} + +func finalQRItem(t *testing.T, ch <-chan QRChannelItem) QRChannelItem { + t.Helper() + select { + case item, open := <-ch: + if !open { + t.Fatal("QR output closed before expected item") + } + return item + case <-time.After(2 * time.Second): + t.Fatal("QR output barrier timed out") + return QRChannelItem{} + } +} +func finalQRClosed(t *testing.T, ch <-chan QRChannelItem) { + t.Helper() + select { + case _, open := <-ch: + if open { + t.Fatal("unexpected item after terminal") + } + case <-time.After(2 * time.Second): + t.Fatal("QR output did not close") + } +} + +func TestFinalQRFinalCloseJoinsHandlerRemoval(t *testing.T) { + c := NewClient(&store.Device{}, nil) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + qr, err := c.GetQRChannel(ctx) + if err != nil { + t.Fatal(err) + } + // The output may close before removal, but FinalClose must wait for the + // reserved cleanup that is blocked by this actual handler-list reader. + c.eventHandlersLock.RLock() + held := true + defer func() { + if held { + c.eventHandlersLock.RUnlock() + } + }() + cancel() + finalQRClosed(t, qr) + finalTestBlocked(t, c) + c.eventHandlersLock.RUnlock() + held = false + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("joined cleanup: %v", err) + } + c.eventHandlersLock.RLock() + count := len(c.eventHandlers) + c.eventHandlersLock.RUnlock() + if count != 0 { + t.Fatalf("handler remains after join: %d", count) + } +} + +type finalQRBarrierLog struct { + waLog.Logger + prefix string + entered, release chan struct{} + once sync.Once + onSub bool +} + +func (l *finalQRBarrierLog) Sub(module string) waLog.Logger { + if l.onSub && module == "QRChannel" { + l.once.Do(func() { close(l.entered); <-l.release }) + } + return l +} +func (l *finalQRBarrierLog) Debugf(msg string, args ...any) { + if strings.HasPrefix(msg, l.prefix) && !l.onSub { + l.once.Do(func() { close(l.entered); <-l.release }) + } +} + +func TestFinalQRLaunchLosesToFinalSeal(t *testing.T) { + log := &finalQRBarrierLog{Logger: waLog.Noop, prefix: "Received QR code event", entered: make(chan struct{}), release: make(chan struct{})} + c := NewClient(&store.Device{}, log) + qr, err := c.GetQRChannel(context.Background()) + if err != nil { + t.Fatal(err) + } + c.startNative(func() { c.dispatchEvent(&events.QR{Codes: []string{"synthetic-code"}}) }) + finalTestWait(t, log.entered) + finalTestBlocked(t, c) + close(log.release) + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } + finalQRClosed(t, qr) + if _, err := c.GetQRChannel(context.Background()); !errors.Is(err, ErrClientClosed) { + t.Fatalf("late QR lifetime: %v", err) + } +} + +func TestFinalQRRegistrationSealStillOwnsCleanup(t *testing.T) { + log := &finalQRBarrierLog{Logger: waLog.Noop, onSub: true, entered: make(chan struct{}), release: make(chan struct{})} + c := NewClient(&store.Device{}, log) + result := make(chan error, 1) + go func() { _, err := c.GetQRChannel(context.Background()); result <- err }() + finalTestWait(t, log.entered) + finalTestBlocked(t, c) + close(log.release) + if err := <-result; !errors.Is(err, ErrClientClosed) { + t.Fatalf("sealed registration returned ready: %v", err) + } + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("registered cleanup: %v", err) + } + c.eventHandlersLock.RLock() + count := len(c.eventHandlers) + c.eventHandlersLock.RUnlock() + if count != 0 { + t.Fatalf("late registration leaked handler: %d", count) + } +} + +func TestFinalQRTerminalStatusesAndPasskeyPayloadsPreserved(t *testing.T) { + cases := []struct { + name string + event any + want QRChannelItem + }{ + {"success", &events.PairSuccess{}, QRChannelSuccess}, + {"timeout", &events.Disconnected{}, QRChannelTimeout}, + {"outdated", &events.ClientOutdated{}, QRChannelClientOutdated}, + {"unexpected-connected", &events.Connected{}, QRChannelErrUnexpectedEvent}, + {"unexpected-failure", &events.ConnectFailure{}, QRChannelErrUnexpectedEvent}, + {"unexpected-logout", &events.LoggedOut{}, QRChannelErrUnexpectedEvent}, + {"unexpected-ban", &events.TemporaryBan{}, QRChannelErrUnexpectedEvent}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c := NewClient(&store.Device{}, nil) + qr, err := c.GetQRChannel(context.Background()) + if err != nil { + t.Fatal(err) + } + c.dispatchEvent(tc.event) + item := finalQRItem(t, qr) + if item != tc.want { + t.Fatalf("status changed: %s", item.Event) + } + finalQRClosed(t, qr) + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } + }) + } + c := NewClient(&store.Device{}, nil) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + qr, err := c.GetQRChannel(ctx) + if err != nil { + t.Fatal(err) + } + c.dispatchEvent(&events.QRScannedWithoutMultidevice{}) + if finalQRItem(t, qr) != QRChannelScannedWithoutMultidevice { + t.Fatal("scan status changed") + } + request := &events.PairPasskeyRequest{} + c.dispatchEvent(request) + if item := finalQRItem(t, qr); item.Event != QRChannelEventPasskeyRequest || item.PasskeyRequest != request { + t.Fatal("passkey request payload changed") + } + confirmation := &events.PairPasskeyConfirmation{} + c.dispatchEvent(confirmation) + if item := finalQRItem(t, qr); item.Event != QRChannelEventPasskeyResponse || item.PasskeyConfirmation != confirmation { + t.Fatal("passkey response payload changed") + } + failure := errors.New("synthetic pairing refusal") + c.dispatchEvent(&events.PairPasskeyError{Error: failure}) + if item := finalQRItem(t, qr); item.Event != QRChannelEventError || item.Error != failure { + t.Fatal("passkey error changed") + } + c.dispatchEvent(&events.PairError{Error: failure}) + if item := finalQRItem(t, qr); item.Event != QRChannelEventError || item.Error != failure { + t.Fatal("pair error changed") + } + finalQRClosed(t, qr) + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } +} + +func TestFinalQRReusableDisconnectAndReplacementIsolation(t *testing.T) { + c := NewClient(&store.Device{}, nil) + qr, err := c.GetQRChannel(context.Background()) + if err != nil { + t.Fatal(err) + } + c.dispatchEvent(&events.QR{Codes: []string{"first"}}) + if item := finalQRItem(t, qr); item.Code != "first" || item.Timeout != 20*time.Second { + t.Fatal("single-code timing changed") + } + c.Disconnect() + select { + case <-qr: + t.Fatal("ordinary Disconnect closed active QR lifetime") + default: + } + c.resetExpectedDisconnect() + c.dispatchEvent(&events.QR{Codes: []string{"second", "3", "4", "5", "6", "7"}}) + if item := finalQRItem(t, qr); item.Code != "second" || item.Timeout != 60*time.Second { + t.Fatal("six-code timing changed") + } + c.dispatchEvent(&events.PairSuccess{}) + if finalQRItem(t, qr) != QRChannelSuccess { + t.Fatal("pair success changed") + } + finalQRClosed(t, qr) + // Cleanup of the old handler must not remove a newly registered handler. + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + replacement, err := c.GetQRChannel(ctx) + if err != nil { + t.Fatal(err) + } + c.dispatchEvent(&events.QRScannedWithoutMultidevice{}) + if finalQRItem(t, replacement) != QRChannelScannedWithoutMultidevice { + t.Fatal("old cleanup removed replacement handler") + } + cancel() + finalQRClosed(t, replacement) + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } + // A retired QR socket pointer has no effect on a replacement incarnation. + other := NewClient(&store.Device{}, nil) + retired, current := &socket.NoiseSocket{}, &socket.NoiseSocket{} + other.socket = current + other.disconnectQRSocket(retired) + if other.socket != current || other.isExpectedDisconnect() { + t.Fatal("retired QR cleanup changed replacement socket") + } + other.disconnectQRSocket(nil) + if other.socket != current { + t.Fatal("unbound QR cleanup changed socket") + } + other.socket = nil // Synthetic identity-only fixture, never connected. + if err := other.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("other close: %v", err) + } +} + +func TestFinalQRBackpressureAndConcurrentTerminalCleanup(t *testing.T) { + c := NewClient(&store.Device{}, nil) + qr, err := c.GetQRChannel(context.Background()) + if err != nil { + t.Fatal(err) + } + c.eventHandlersLock.RLock() + id := c.eventHandlers[0].id + c.eventHandlersLock.RUnlock() + for i := 0; i < 8; i++ { + c.dispatchEvent(&events.QRScannedWithoutMultidevice{}) + } + c.dispatchEvent(&events.QR{Codes: []string{"backpressured"}}) + // Wait for the exact QR lifetime to close before draining: nonblocking code + // emission must preserve the original full-buffer shutdown behavior. + // A terminal event races the admitted emitter; both must share one close. + done := make(chan struct{}) + go func() { c.dispatchEvent(&events.PairSuccess{}); close(done) }() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } + finalTestWait(t, done) + for range qr { + } + if c.RemoveEventHandler(id) { + t.Fatal("owned QR handler was not removed") + } +} + +func TestFinalQRBackpressureActuallyClosesWithoutExtraCode(t *testing.T) { + log := &finalQRBarrierLog{Logger: waLog.Noop, prefix: "Output channel didn't accept code", entered: make(chan struct{}), release: make(chan struct{})} + c := NewClient(&store.Device{}, log) + qr, err := c.GetQRChannel(context.Background()) + if err != nil { + t.Fatal(err) + } + for i := 0; i < 8; i++ { + c.dispatchEvent(&events.QRScannedWithoutMultidevice{}) + } + c.dispatchEvent(&events.QR{Codes: []string{"never-queued"}}) + finalTestWait(t, log.entered) + close(log.release) + count := 0 + for { + select { + case item, open := <-qr: + if !open { + if count != 8 { + t.Fatalf("buffer count changed: %d", count) + } + goto drained + } + if item != QRChannelScannedWithoutMultidevice { + t.Fatal("backpressured code was delivered") + } + count++ + case <-time.After(2 * time.Second): + t.Fatal("backpressure cleanup did not close output") + } + } +drained: + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } +} + +func TestFinalQREmptyBatchTimesOutAndCancelledSetupRefuses(t *testing.T) { + c := NewClient(&store.Device{}, nil) + cancelled, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := c.GetQRChannel(cancelled); !errors.Is(err, context.Canceled) { + t.Fatalf("cancelled setup: %v", err) + } + qr, err := c.GetQRChannel(context.Background()) + if err != nil { + t.Fatal(err) + } + c.dispatchEvent(&events.QR{}) + if finalQRItem(t, qr) != QRChannelTimeout { + t.Fatal("empty batch timeout changed") + } + finalQRClosed(t, qr) + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } +} + +func TestFinalQRCancellingOldClientDoesNotAffectNewClient(t *testing.T) { + old, fresh := NewClient(&store.Device{}, nil), NewClient(&store.Device{}, nil) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + oldQR, err := old.GetQRChannel(ctx) + if err != nil { + t.Fatal(err) + } + freshQR, err := fresh.GetQRChannel(context.Background()) + if err != nil { + t.Fatal(err) + } + cancel() + finalQRClosed(t, oldQR) + fresh.dispatchEvent(&events.PairSuccess{}) + if finalQRItem(t, freshQR) != QRChannelSuccess { + t.Fatal("old cancellation changed replacement lifetime") + } + finalQRClosed(t, freshQR) + for _, c := range []*Client{old, fresh} { + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("close: %v", err) + } + } +} diff --git a/qrchan.go b/qrchan.go index f24bfea87..8cf3b6be5 100644 --- a/qrchan.go +++ b/qrchan.go @@ -14,6 +14,7 @@ import ( "sync/atomic" "time" + "github.com/polymorfa/hypermeow/socket" "github.com/polymorfa/hypermeow/types/events" waLog "github.com/polymorfa/hypermeow/util/log" ) @@ -56,69 +57,138 @@ var ( type qrChannel struct { sync.Mutex - cli *Client - log waLog.Logger - ctx context.Context - handlerID uint32 - closed atomic.Bool - output chan<- QRChannelItem - stopQRs chan struct{} + cli *Client + log waLog.Logger + ctx context.Context + handlerID uint32 + closed atomic.Bool + output chan<- QRChannelItem + stopQRs chan struct{} + sendsDone chan struct{} + outputClosed chan struct{} + senders int + terminal *QRChannelItem + disconnect bool + boundSocket *socket.NoiseSocket } -func (qrc *qrChannel) close() bool { - return !qrc.closed.Swap(true) +// requestClose seals sends once. Only the reserved cleanup worker closes output +// or removes the handler. It never waits while holding QR/client state locks. +func (qrc *qrChannel) requestClose(item *QRChannelItem, disconnect bool, expected *socket.NoiseSocket) bool { + qrc.Lock() + defer qrc.Unlock() + if qrc.closed.Load() { + return false + } + qrc.terminal, qrc.disconnect, qrc.boundSocket = item, disconnect, expected + qrc.closed.Store(true) + close(qrc.stopQRs) + if qrc.senders == 0 { + close(qrc.sendsDone) + } + return true } -func (qrc *qrChannel) emitQRs(codes []string) { - var nextCode string +func (qrc *qrChannel) send(item QRChannelItem, nonblocking bool) bool { + qrc.Lock() + if qrc.closed.Load() { + qrc.Unlock() + return false + } + qrc.senders++ + qrc.Unlock() + defer func() { + qrc.Lock() + qrc.senders-- + if qrc.closed.Load() && qrc.senders == 0 { + close(qrc.sendsDone) + } + qrc.Unlock() + }() + if nonblocking { + select { + case qrc.output <- item: + return true + case <-qrc.stopQRs: + return false + case <-qrc.ctx.Done(): + return false + default: + return false + } + } + select { + case qrc.output <- item: + return true + case <-qrc.stopQRs: + return false + case <-qrc.ctx.Done(): + return false + } +} + +func (qrc *qrChannel) run() { + select { + case <-qrc.stopQRs: + case <-qrc.ctx.Done(): + qrc.Lock() + expected := qrc.boundSocket + qrc.Unlock() + qrc.requestClose(nil, expected != nil, expected) + } + <-qrc.sendsDone + qrc.Lock() + terminal, disconnect, expected := qrc.terminal, qrc.disconnect, qrc.boundSocket + qrc.Unlock() + if disconnect && !qrc.cli.nativeClosed() { + qrc.cli.disconnectQRSocket(expected) + } + if terminal != nil && qrc.ctx.Err() == nil { + select { + case qrc.output <- *terminal: + case <-qrc.ctx.Done(): + } + } + close(qrc.output) + // Wake terminal callbacks before acquiring eventHandlersLock for removal. + // They are running under its read lock and must be allowed to return first. + close(qrc.outputClosed) + qrc.cli.RemoveEventHandler(qrc.handlerID) +} + +func (qrc *qrChannel) emitQRs(codes []string, expected *socket.NoiseSocket) { for { if len(codes) == 0 { - if qrc.close() { - qrc.log.Debugf("Ran out of QR codes, closing channel with status %s and disconnecting client", QRChannelTimeout) - qrc.output <- QRChannelTimeout - close(qrc.output) - go qrc.cli.RemoveEventHandler(qrc.handlerID) - qrc.cli.Disconnect() - } else { - qrc.log.Debugf("Ran out of QR codes, but channel is already closed") - } + qrc.requestClose(&QRChannelTimeout, true, expected) return } else if qrc.closed.Load() { - qrc.log.Debugf("QR code channel is closed, exiting QR emitter") return } timeout := 20 * time.Second if len(codes) == 6 { timeout = 60 * time.Second } - nextCode, codes = codes[0], codes[1:] + nextCode := codes[0] + codes = codes[1:] qrc.log.Debugf("Emitting QR code %s", nextCode) - select { - case qrc.output <- QRChannelItem{Code: nextCode, Timeout: timeout, Event: QRChannelEventCode}: - default: + if !qrc.send(QRChannelItem{Code: nextCode, Timeout: timeout, Event: QRChannelEventCode}, true) { qrc.log.Debugf("Output channel didn't accept code, exiting QR emitter") - if qrc.close() { - close(qrc.output) - go qrc.cli.RemoveEventHandler(qrc.handlerID) - qrc.cli.Disconnect() - } + qrc.requestClose(nil, true, expected) return } + timer := time.NewTimer(timeout) select { - case <-time.After(timeout): + case <-timer.C: case <-qrc.stopQRs: - qrc.log.Debugf("Got signal to stop QR emitter") + timer.Stop() return case <-qrc.cli.expectedDisconnect.GetChan(): - qrc.log.Debugf("Client is expected to disconnect, stopping QR emitter") + timer.Stop() return case <-qrc.ctx.Done(): - qrc.log.Debugf("Context is done, stopping QR emitter") - if qrc.close() { - close(qrc.output) - go qrc.cli.RemoveEventHandler(qrc.handlerID) - qrc.cli.Disconnect() - } + timer.Stop() + qrc.requestClose(nil, true, expected) + return } } } @@ -132,40 +202,51 @@ func (qrc *qrChannel) handleEvent(rawEvt any) { switch evt := rawEvt.(type) { case *events.QR: qrc.log.Debugf("Received QR code event, starting to emit codes to channel") - go qrc.emitQRs(slices.Clone(evt.Codes)) + codes := slices.Clone(evt.Codes) + qrc.cli.socketLock.RLock() + expected := qrc.cli.socket + qrc.cli.socketLock.RUnlock() + qrc.Lock() + if !qrc.closed.Load() { + qrc.boundSocket = expected + // QR-local closure and emitter admission share this lock. The + // helper only reserves/schedules; it never waits for worker exit. + qrc.cli.startNativeContext(qrc.ctx, func() { qrc.emitQRs(codes, expected) }) + } + qrc.Unlock() return case *events.QRScannedWithoutMultidevice: qrc.log.Debugf("QR code scanned without multidevice enabled") - qrc.output <- QRChannelScannedWithoutMultidevice + qrc.send(QRChannelScannedWithoutMultidevice, false) return case *events.PairPasskeyRequest: - qrc.output <- QRChannelItem{ + qrc.send(QRChannelItem{ Event: QRChannelEventPasskeyRequest, PasskeyRequest: evt, - } + }, false) return case *events.PairPasskeyConfirmation: if evt.SkipHandoffUX { qrc.log.Debugf("Sending automatic passkey confirmation") err := qrc.cli.SendPasskeyConfirmation(qrc.ctx) if err != nil { - qrc.output <- QRChannelItem{ + qrc.send(QRChannelItem{ Event: QRChannelEventError, Error: fmt.Errorf("failed to send passkey confirmation automatically: %w", err), - } + }, false) } } else { - qrc.output <- QRChannelItem{ + qrc.send(QRChannelItem{ Event: QRChannelEventPasskeyResponse, PasskeyConfirmation: evt, - } + }, false) } return case *events.PairPasskeyError: - qrc.output <- QRChannelItem{ + qrc.send(QRChannelItem{ Event: QRChannelEventError, Error: evt.Error, - } + }, false) return case *events.ClientOutdated: outputType = QRChannelClientOutdated @@ -183,40 +264,82 @@ func (qrc *qrChannel) handleEvent(rawEvt any) { default: return } - close(qrc.stopQRs) - if qrc.close() { - qrc.log.Debugf("Closing channel with status %+v", outputType) - qrc.output <- outputType - close(qrc.output) - } else { - qrc.log.Debugf("Got status %+v, but channel is already closed", outputType) - } - // Has to be done in background because otherwise there's a deadlock with eventHandlersLock - go qrc.cli.RemoveEventHandler(qrc.handlerID) + qrc.requestClose(&outputType, false, nil) + select { + case <-qrc.outputClosed: + case <-qrc.ctx.Done(): + } +} + +// QR cancellation/expiry belongs to the socket observed by that QR emitter, +// not a later socket on the same Client. No socket identity means no disconnect +// authority. Cleanup of pending requests stays inside this exact-pointer guard. +func (cli *Client) disconnectQRSocket(expected *socket.NoiseSocket) { + if expected == nil { + return + } + cli.socketLock.Lock() + defer cli.socketLock.Unlock() + if cli.socket != expected { + return + } + cli.expectDisconnect() + cli.unlockedDisconnect() + cli.clearDelayedMessageRequests() } // GetQRChannel returns a channel that automatically outputs a new QR code when the previous one expires. // // This must be called *before* Connect(). It will then listen to all the relevant events from the client. // -// The last value to be emitted will be a special event like "success", "timeout" or another error code -// depending on the result of the pairing. The channel will be closed immediately after one of those. +// Pairing terminal events emit a special item such as "success", "timeout" or +// an error, then close the channel. Caller cancellation, final client close or +// output backpressure can close it without a terminal pairing item. Output close +// alone does not confirm successful pairing or complete client termination. func (cli *Client) GetQRChannel(ctx context.Context) (<-chan QRChannelItem, error) { if cli == nil { return nil, ErrClientIsNil - } else if cli.IsConnected() { + } + if err := ctx.Err(); err != nil { + return nil, err + } + done, admitted := cli.enterNative() + if !admitted { + return nil, ErrClientClosed + } + launched := false + defer func() { + if !launched { + done() + } + }() + if cli.IsConnected() { return nil, ErrQRAlreadyConnected - } else if cli.Store.ID != nil { + } + if cli.Store.ID != nil { return nil, ErrQRStoreContainsID } + ctx, release := cli.nativeContext(ctx) + defer func() { + if !launched { + release() + } + }() ch := make(chan QRChannelItem, 8) - qrc := qrChannel{ - output: ch, - stopQRs: make(chan struct{}), - cli: cli, - log: cli.Log.Sub("QRChannel"), - ctx: ctx, + qrc := &qrChannel{ + output: ch, stopQRs: make(chan struct{}), sendsDone: make(chan struct{}), outputClosed: make(chan struct{}), + cli: cli, log: cli.Log.Sub("QRChannel"), ctx: ctx, } qrc.handlerID = cli.AddEventHandler(qrc.handleEvent) + // This owner was recorded before handler registration, not re-admitted after + // it. Even a final seal during registration must join its exact cleanup. + launched = true + go func() { defer done(); defer release(); qrc.run() }() + if cli.nativeClosed() { + return nil, ErrClientClosed + } + if err := ctx.Err(); err != nil { + return nil, err + } return ch, nil } From 8a4cf3ea0b24d7b610ab6947ad77b5e5d1398f02 Mon Sep 17 00:00:00 2001 From: Rajeh Taher Date: Thu, 22 Oct 2026 07:21:44 +0300 Subject: [PATCH 04/10] Synchronize forwarded cancellation test and always release fixture --- connection_lifecycle_test.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/connection_lifecycle_test.go b/connection_lifecycle_test.go index 6477c7a18..bd666c1c5 100644 --- a/connection_lifecycle_test.go +++ b/connection_lifecycle_test.go @@ -65,6 +65,18 @@ type finalConnectionContextKey struct{} func TestFinalConnectionSuccessJoinsBlockedOwnedTask(t *testing.T) { blocking := &finalConnectionPrekeys{entered: make(chan context.Context, 1), release: make(chan struct{})} c := NewClient(&store.Device{PreKeys: blocking}, nil) + var releaseOnce sync.Once + release := func() { releaseOnce.Do(func() { close(blocking.release) }) } + // Register before any assertion so failure cannot strand the native task + // or its teardown. The explicit release below and cleanup share one close. + t.Cleanup(func() { + release() + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + if err := c.FinalClose(ctx); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Errorf("post-connect cleanup: %v", err) + } + }) c.paired.Store(true) c.lastTCTokenDBPrune = time.Now() // Do not launch the separately unbound prune. parent := context.WithValue(context.Background(), finalConnectionContextKey{}, "preserved-value") @@ -79,10 +91,12 @@ func TestFinalConnectionSuccessJoinsBlockedOwnedTask(t *testing.T) { t.Fatal("event context value lost") } finalTestBlocked(t, c) + // Root cancellation does not order the asynchronous forwarding callback. + finalTestWait(t, inherited.Done()) if !errors.Is(inherited.Err(), context.Canceled) { t.Fatal("post-connect task did not receive final cancellation") } - close(blocking.release) + release() if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { t.Fatalf("post-connect join: %v", err) } From 0767ab36055392c9d957268fdd43e32cb4aa77c3 Mon Sep 17 00:00:00 2001 From: Rajeh Taher Date: Thu, 22 Oct 2026 07:40:15 +0300 Subject: [PATCH 05/10] Own notification and privacy background task lifetimes --- CLIENT_LIFETIME.md | 45 ++- client.go | 13 +- notification.go | 40 ++- notification_privacy_lifecycle_test.go | 394 +++++++++++++++++++++++++ pair.go | 2 +- presence.go | 2 +- send.go | 2 +- tctoken.go | 22 +- 8 files changed, 490 insertions(+), 30 deletions(-) create mode 100644 notification_privacy_lifecycle_test.go diff --git a/CLIENT_LIFETIME.md b/CLIENT_LIFETIME.md index 49dc61a6b..2d8192e01 100644 --- a/CLIENT_LIFETIME.md +++ b/CLIENT_LIFETIME.md @@ -39,6 +39,9 @@ the deadline ends its waiter and does not manufacture completion. starts before Connect, including when no QR event ever arrives. Caller/final cancellation ends that lifetime. Ordinary Disconnect stops an emitter as before and leaves the channel available for subsequent events. +- Async notification dispatches, server AB-props tasks and their ACK callback, + privacy-token issuance/persistence, unified-session children and expired-token + pruning, including the connect-success prune. See the exact boundaries below. - One teardown task, whose completion is recorded before launch. Context forwarding uses context.AfterFunc only to cancel contexts, with no user/protocol callbacks; those small cancellation callbacks are not a joined product-worker claim. @@ -102,10 +105,40 @@ responsibility. No pairing/protocol/Signal/key/store body or payload changed. The existing automatic passkey-confirmation method is invoked synchronously by its admitted QR callback, not reclassified as an accepted Signal producer. -Notifications/AB-props and privacy pruning were deliberately not folded into -this group. Connection-success's pruning call still reaches an unbound task as -listed below. External RefreshCAT implementations, store/transport internals and -user-spawned callbacks do not gain owned completion proof from these wrappers. +## Notification and privacy task ownership + +The five async notification dispatches (encrypt, server_sync, account_sync, +link_code_companion_reg and crsc_continuation) register before scheduling and +own their forwarding contexts until the called handler returns. Their existing +handler bodies remain unchanged, apart from the encrypt handler's privacy child +launch. Child producers inside those handlers still require the boundaries below. + +Server AB-props tasks retain the original two-minute bound and pending-ID +coalescing. Refused admission removes the reserved pending entry. Task exit also +removes it on success, failure or cancellation. ACK admission checks the final +seal independently of asynchronous context forwarding, and the task counts the +actual ACK callback until return. Refusal withholds ACK; it never reports sync +success. No pending-map lock is held during the handler or completion wait. + +Privacy issuance/persistence and unified-session children keep their original +BackgroundEventCtx ancestry and values. They now derive a forwarding context for +final cancellation and stay counted through the actual IQ/store/send return, +including a store or transport that ignores cancellation. The synchronous +DangerousInternals entry points keep their original signatures and Background +semantics; they remain caller-owned. Both presence and pairing's unified child +launches are gated, but the outer pairing producer and PrePairCallback remain +unbound. No pairing body or protocol payload changed. + +Expired-token pruning, including the connect-success child, registers before +launch. Its existing TryLock guard coalesces work until the store actually +returns. Refused admission releases that guard without consuming the prune +timestamp. An admitted task records the original captured timestamp under the +guard. The 24-hour interval, cutoff algorithm and store body are unchanged. +Ordinary Disconnect does not seal or cancel these background tasks. + +External RefreshCAT implementations, store/transport internals and user-spawned +callbacks do not gain whole-client completion proof from these wrappers. The +established-context retention described above remains conservative and unchanged. ## Still unbound @@ -118,12 +151,10 @@ owning-library work before any successful stop result is possible. | Source at base 29a42459 | Unbound lifetime | | --- | --- | -| notification.go:57,522-566,603 | Privacy token, encryption/app-state/account-sync, code/passkey pairing, and server AB-props sync tasks. | | message.go:82-89,373-374,469-470,520,549,770,887,1005-1037 | Name updates, retry/ack work, event-buffer cleanup, event-dispatch wrapper, history download/storage/notification loops, key-share and peer-data work. | | appstate.go:162,645,659 | Missing-key request and async event batches. | -| pair.go:138-147 | Pair completion, PrePairCallback and unified-session child task. | +| pair.go:138-147 | Outer pair completion and PrePairCallback. Its unified-session child is now tracked; the parent and its other boundaries are not. | | receipt.go:27,36,109,128; retry.go:469,541 | Async ack/retry handlers and delayed phone rerequests. Cancellation of delayed requests is not their join. | -| presence.go:71; send.go:903; tctoken.go:150 | Unified-session, privacy-token persistence and expired-token pruning. The prune called synchronously by connect-success still launches this separate unbound task. | | handshake.go:98-103 | GetClientPayload callback admission is not separately sealed; an admitted connection can reach it. The enclosing connect is counted, which is not full callback-admission coverage. | Caller-invoked public API methods, DangerousInternals, external HTTP transports, diff --git a/client.go b/client.go index 8895e986f..a8cc1ad59 100644 --- a/client.go +++ b/client.go @@ -1271,10 +1271,21 @@ func (cli *Client) getUnifiedSessionID() string { return strconv.FormatInt(unifiedID, 10) } +func (cli *Client) startUnifiedSession() bool { + return cli.startConnectionTask(cli.BackgroundEventCtx, cli.sendUnifiedSessionContext) +} + func (cli *Client) sendUnifiedSession() { if cli == nil { return } + cli.sendUnifiedSessionContext(cli.BackgroundEventCtx) +} + +func (cli *Client) sendUnifiedSessionContext(ctx context.Context) { + if cli == nil { + return + } node := waBinary.Node{ Tag: "ib", @@ -1287,7 +1298,7 @@ func (cli *Client) sendUnifiedSession() { }}, } - err := cli.sendNode(cli.BackgroundEventCtx, node) + err := cli.sendNode(ctx, node) if err != nil { cli.Log.Debugf("Failed to send unified_session telemetry: %v", err) } diff --git a/notification.go b/notification.go index 987beeea8..dd5c1dcf1 100644 --- a/notification.go +++ b/notification.go @@ -54,7 +54,7 @@ func (cli *Client) handleEncryptNotification(ctx context.Context, node *waBinary senderTS := cli.getTCTokenSenderTS(storageLID) if !senderTS.IsZero() { cli.Log.Debugf("Identity changed for %s, re-issuing tctoken", from) - go cli.issuePrivacyTokenAndSave(storageLID, senderTS) + cli.startPrivacyTokenIssue(storageLID, senderTS) } } cli.dispatchEvent(&events.IdentityChange{JID: from, Timestamp: ts}) @@ -519,11 +519,11 @@ func (cli *Client) handleNotification(ctx context.Context, node *waBinary.Node) } switch notifType { case "encrypt": - go cli.handleEncryptNotification(ctx, node) + cli.startConnectionTask(ctx, func(ctx context.Context) { cli.handleEncryptNotification(ctx, node) }) case "server_sync": - go cli.handleAppStateNotification(ctx, node) + cli.startConnectionTask(ctx, func(ctx context.Context) { cli.handleAppStateNotification(ctx, node) }) case "account_sync": - go cli.handleAccountSyncNotification(ctx, node) + cli.startConnectionTask(ctx, func(ctx context.Context) { cli.handleAccountSyncNotification(ctx, node) }) case "devices": cli.handleDeviceNotification(ctx, node) case "fbid:devices": @@ -551,7 +551,7 @@ func (cli *Client) handleNotification(ctx context.Context, node *waBinary.Node) case "privacy_token": cli.handlePrivacyTokenNotification(ctx, node) case "link_code_companion_reg": - go cli.tryHandleCodePairNotification(ctx, node) + cli.startConnectionTask(ctx, func(ctx context.Context) { cli.tryHandleCodePairNotification(ctx, node) }) case "newsletter": cli.handleNewsletterNotification(ctx, node) case "mex": @@ -563,7 +563,7 @@ func (cli *Client) handleNotification(ctx context.Context, node *waBinary.Node) case "business": cli.handleQueuedBusinessCatalogNotification(node) case "crsc_continuation": - go cli.tryHandlePasskeyContinuationNotification(ctx, node) + cli.startConnectionTask(ctx, func(ctx context.Context) { cli.tryHandlePasskeyContinuationNotification(ctx, node) }) // Other types: disappearing_mode, server, status, pay, psa default: cli.Log.Debugf("Unhandled notification with type %s", notifType) @@ -600,21 +600,33 @@ func (cli *Client) deferServerABProps(ctx context.Context, node *waBinary.Node, } cli.serverABPropsPending[key] = struct{}{} cli.serverABPropsMu.Unlock() - go func() { + cleanup := func() { + cli.serverABPropsMu.Lock() + delete(cli.serverABPropsPending, key) + cli.serverABPropsMu.Unlock() + } + if !cli.startConnectionTask(ctx, func(ctx context.Context) { // Allow three 30-second attempts, two retry waits and durable completion. taskCtx, cancel := context.WithTimeout(ctx, 2*time.Minute) defer cancel() - defer func() { - cli.serverABPropsMu.Lock() - delete(cli.serverABPropsPending, key) - cli.serverABPropsMu.Unlock() - }() + defer cleanup() if ctx.Err() != nil { return } if err := cli.ServerABPropsSync(taskCtx); err == nil && taskCtx.Err() == nil { - ack() + // Admission orders the ACK against the final seal even when context + // cancellation forwarding has not run yet. Join the actual callback. + done, admitted := cli.enterNative() + if !admitted { + return + } + defer done() + if taskCtx.Err() == nil { + ack() + } } - }() + }) { + cleanup() + } return true } diff --git a/notification_privacy_lifecycle_test.go b/notification_privacy_lifecycle_test.go new file mode 100644 index 000000000..62ad28a15 --- /dev/null +++ b/notification_privacy_lifecycle_test.go @@ -0,0 +1,394 @@ +package whatsmeow + +import ( + "context" + "errors" + waBinary "github.com/polymorfa/hypermeow/binary" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/polymorfa/hypermeow/store" + "github.com/polymorfa/hypermeow/types" + waLog "github.com/polymorfa/hypermeow/util/log" +) + +type finalPrivacyStore struct { + store.NoopStore + entered chan context.Context + release chan struct{} +} + +func (s *finalPrivacyStore) DeleteExpiredPrivacyTokens(ctx context.Context, _ time.Time) (int64, error) { + s.entered <- ctx + <-s.release // Deliberately ignore cancellation: completion must still be joined. + return 0, nil +} +func (s *finalPrivacyStore) GetPrivacyToken(context.Context, types.JID) (*store.PrivacyToken, error) { + return nil, nil +} +func (s *finalPrivacyStore) PutPrivacyTokens(context.Context, ...store.PrivacyToken) error { + return nil +} + +func finalPrivacyFixture(t *testing.T) (*Client, *finalPrivacyStore, func()) { + t.Helper() + s := &finalPrivacyStore{entered: make(chan context.Context, 1), release: make(chan struct{})} + c := NewClient(&store.Device{PrivacyTokens: s}, nil) + var once sync.Once + release := func() { once.Do(func() { close(s.release) }) } + t.Cleanup(func() { + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Errorf("cleanup: %v", err) + } + }) + return c, s, release +} +func finalPrivacyEntered(t *testing.T, s *finalPrivacyStore) context.Context { + t.Helper() + select { + case ctx := <-s.entered: + return ctx + case <-time.After(2 * time.Second): + t.Fatal("store not reached") + return nil + } +} +func TestFinalPrivacyPruneJoinsActualStore(t *testing.T) { + c, s, release := finalPrivacyFixture(t) + c.deleteExpiredPrivacyTokens() + inherited := finalPrivacyEntered(t, s) + c.Disconnect() + if inherited.Err() != nil { + t.Fatal("ordinary disconnect cancelled prune") + } + finalTestBlocked(t, c) + finalTestWait(t, inherited.Done()) + if !errors.Is(inherited.Err(), context.Canceled) { + t.Fatal("missing final cancellation") + } + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("join: %v", err) + } +} + +func TestFinalPrivacyPruneRefusalAndReplacement(t *testing.T) { + c, s, _ := finalPrivacyFixture(t) + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + c.deleteExpiredPrivacyTokens() + if !c.tcTokenDBPruneLock.TryLock() { + t.Fatal("refused prune stranded mutex") + } + if !c.lastTCTokenDBPrune.IsZero() { + t.Error("refused launch consumed prune timestamp") + } + c.tcTokenDBPruneLock.Unlock() + select { + case <-s.entered: + t.Fatal("store called after seal") + default: + } + replacement, rs, release := finalPrivacyFixture(t) + replacement.deleteExpiredPrivacyTokens() + ctx := finalPrivacyEntered(t, rs) + if ctx.Err() != nil { + t.Fatal("replacement cancelled by retired client") + } + release() +} + +type finalPrivacyPrekeys struct{ store.NoopStore } + +func (*finalPrivacyPrekeys) UploadedPreKeyCount(context.Context) (int, error) { + return 0, errors.New("synthetic inventory refusal") +} +func TestFinalPrivacyConnectSuccessPruneChild(t *testing.T) { + c, s, release := finalPrivacyFixture(t) + c.Store.PreKeys = &finalPrivacyPrekeys{} + c.paired.Store(true) + c.handleConnectSuccess(context.Background(), &waBinary.Node{Tag: "success", Attrs: waBinary.Attrs{"t": "1"}}) + ctx := finalPrivacyEntered(t, s) + finalTestBlocked(t, c) + finalTestWait(t, ctx.Done()) + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } +} + +func finalABPropsPending(c *Client) int { + c.serverABPropsMu.Lock() + defer c.serverABPropsMu.Unlock() + return len(c.serverABPropsPending) +} +func TestFinalNotificationABPropsJoinsAndCleans(t *testing.T) { + for _, final := range []bool{false, true} { + t.Run(map[bool]string{false: "caller-cancel", true: "final-seal"}[final], func(t *testing.T) { + c := NewClient(&store.Device{}, nil) + entered := make(chan context.Context, 1) + release := make(chan struct{}) + var once sync.Once + unblock := func() { once.Do(func() { close(release) }) } + t.Cleanup(func() { + unblock() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + var calls, acks atomic.Int32 + c.ServerABPropsSync = func(ctx context.Context) error { calls.Add(1); entered <- ctx; <-release; return nil } + parent, cancel := context.WithCancel(context.WithValue(context.Background(), finalConnectionContextKey{}, "notification-value")) + defer cancel() + c.deferServerABProps(parent, abpropsNotification(), func() { acks.Add(1) }) + var ctx context.Context + select { + case ctx = <-entered: + case <-time.After(2 * time.Second): + t.Fatal("callback not reached") + } + if ctx.Value(finalConnectionContextKey{}) != "notification-value" { + t.Fatal("caller value lost") + } + if d, ok := ctx.Deadline(); !ok || time.Until(d) > 2*time.Minute { + t.Fatal("2 minute bound lost") + } + c.deferServerABProps(parent, abpropsNotification(), func() { acks.Add(1) }) + if calls.Load() != 1 || finalABPropsPending(c) != 1 { + t.Fatal("dedup lost") + } + c.Disconnect() + if ctx.Err() != nil { + t.Fatal("ordinary disconnect cancelled task") + } + if final { + finalTestBlocked(t, c) + } else { + cancel() + } + finalTestWait(t, ctx.Done()) + if !final { + finalTestBlocked(t, c) + } + unblock() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + if acks.Load() != 0 || finalABPropsPending(c) != 0 { + t.Fatal("cancelled callback acknowledged or pending entry retained") + } + c.deferServerABProps(context.Background(), abpropsNotification(), func() { acks.Add(1) }) + if calls.Load() != 1 || acks.Load() != 0 || finalABPropsPending(c) != 0 { + t.Fatal("refused launch leaked entry or called handler/ACK") + } + if finalContextWatcherCount(c) != 0 { + t.Fatal("task forwarding watcher retained") + } + }) + } +} + +// Synthetic in-process node relay: no provider, socket, credentials or crypto. +type finalPrivacyRelay struct { + *fakeShadowRelay + client *Client + entered chan context.Context + release chan struct{} +} + +func (r *finalPrivacyRelay) SendNode(ctx context.Context, data []byte) error { + unpacked, err := waBinary.Unpack(data) + if err != nil { + return err + } + node, err := waBinary.Unmarshal(unpacked) + if err != nil { + return err + } + if node.Tag == "iq" { + r.client.receiveResponse(ctx, &waBinary.Node{Tag: "iq", Attrs: waBinary.Attrs{"id": node.Attrs["id"], "type": "result"}}) + return nil + } + if node.Tag == "ib" { + r.entered <- ctx + <-r.release + return ctx.Err() + } + return ctx.Err() +} +func TestFinalPrivacyUnifiedBackgroundReuseAndJoin(t *testing.T) { + c := NewClient(&store.Device{PushName: "synthetic"}, nil) + entered := make(chan context.Context, 2) + release := make(chan struct{}) + var once sync.Once + unblock := func() { once.Do(func() { close(release) }) } + t.Cleanup(func() { + unblock() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + c.BackgroundEventCtx = context.WithValue(context.Background(), finalConnectionContextKey{}, "background-value") + c.shadowRelay = &finalPrivacyRelay{fakeShadowRelay: &fakeShadowRelay{}, client: c, entered: entered, release: release} + caller, cancel := context.WithCancel(context.Background()) + cancel() + if err := c.SendPresence(caller, types.PresenceAvailable); !errors.Is(err, context.Canceled) { + t.Fatalf("caller semantics: %v", err) + } + var inherited context.Context + select { + case inherited = <-entered: + case <-time.After(2 * time.Second): + t.Fatal("unified task absent") + } + if inherited.Err() != nil || inherited.Value(finalConnectionContextKey{}) != "background-value" { + t.Fatal("background semantics/value lost") + } + c.Disconnect() + if !c.startUnifiedSession() { + t.Fatal("ordinary disconnect sealed reuse") + } + select { + case ctx := <-entered: + if ctx.Err() != nil { + t.Fatal("reused lifetime cancelled") + } + case <-time.After(2 * time.Second): + t.Fatal("reused task absent") + } + finalTestBlocked(t, c) + finalTestWait(t, inherited.Done()) + if c.startUnifiedSession() { + t.Fatal("unified launch after seal") + } + unblock() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + if finalContextWatcherCount(c) != 0 { + t.Fatal("unified forwarding watcher retained") + } +} + +type finalPrivacyPersistence struct{ *finalPrivacyStore } + +func (s *finalPrivacyPersistence) GetPrivacyToken(context.Context, types.JID) (*store.PrivacyToken, error) { + return &store.PrivacyToken{Token: []byte("synthetic")}, nil +} +func (s *finalPrivacyPersistence) PutPrivacyTokens(ctx context.Context, _ ...store.PrivacyToken) error { + s.entered <- ctx + <-s.release + return nil +} +func TestFinalPrivacyPersistenceJoinsActualStore(t *testing.T) { + c, s, release := finalPrivacyFixture(t) + c.Store.PrivacyTokens = &finalPrivacyPersistence{s} + c.BackgroundEventCtx = context.WithValue(context.Background(), finalConnectionContextKey{}, "privacy-value") + c.shadowRelay = &finalPrivacyRelay{fakeShadowRelay: &fakeShadowRelay{}, client: c} + if !c.startPrivacyTokenIssue(types.NewJID("synthetic", types.HiddenUserServer), time.Unix(123, 0)) { + t.Fatal("privacy task refused") + } + ctx := finalPrivacyEntered(t, s) + if ctx.Value(finalConnectionContextKey{}) != "privacy-value" { + t.Fatal("privacy context value lost") + } + finalTestBlocked(t, c) + finalTestWait(t, ctx.Done()) + if c.startPrivacyTokenIssue(types.EmptyJID, time.Time{}) { + t.Fatal("privacy launch after seal") + } + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + if finalContextWatcherCount(c) != 0 { + t.Fatal("privacy forwarding watcher retained") + } +} + +func TestFinalNotificationABPropsOrdinaryRedelivery(t *testing.T) { + for _, fail := range []bool{false, true} { + t.Run(map[bool]string{false: "success", true: "failure"}[fail], func(t *testing.T) { + c := NewClient(&store.Device{}, nil) + t.Cleanup(func() { + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + contexts := make(chan context.Context, 2) + var calls, acks atomic.Int32 + c.ServerABPropsSync = func(ctx context.Context) error { + calls.Add(1) + contexts <- ctx + if fail { + return errors.New("synthetic sync failure") + } + return nil + } + for i := 0; i < 2; i++ { + c.deferServerABProps(context.Background(), abpropsNotification(), func() { acks.Add(1) }) + var ctx context.Context + select { + case ctx = <-contexts: + case <-time.After(2 * time.Second): + t.Fatal("redelivery not admitted") + } + // This timeout context is cancelled by the callback's defer after pending cleanup. + finalTestWait(t, ctx.Done()) + if finalABPropsPending(c) != 0 { + t.Fatal("completed task retained pending entry") + } + c.Disconnect() + } + if calls.Load() != 2 || (!fail && acks.Load() != 2) || (fail && acks.Load() != 0) { + t.Fatal("ordinary redelivery/ACK semantics changed") + } + }) + } +} + +type finalNotificationLog struct { + waLog.Logger + entered, release chan struct{} + calls atomic.Int32 +} + +func (l *finalNotificationLog) Sub(string) waLog.Logger { return l } +func (l *finalNotificationLog) Debugf(msg string, args ...any) { + if msg == "Parsing privacy settings change notification" { + l.calls.Add(1) + close(l.entered) + <-l.release + } +} +func TestFinalNotificationHandlerActualJoinAndSealRefusal(t *testing.T) { + log := &finalNotificationLog{Logger: waLog.Noop, entered: make(chan struct{}), release: make(chan struct{})} + c := NewClient(&store.Device{}, log) + c.SynchronousAck = true + var once sync.Once + release := func() { once.Do(func() { close(log.release) }) } + t.Cleanup(func() { + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + node := &waBinary.Node{Tag: "notification", Attrs: waBinary.Attrs{"type": "account_sync", "from": types.ServerJID, "id": "synthetic"}, Content: []waBinary.Node{{Tag: "privacy"}}} + c.handleNotification(context.Background(), node) + finalTestWait(t, log.entered) + c.Disconnect() + finalTestBlocked(t, c) + // The second dispatch must be refused even though the first handler is blocked. + c.handleNotification(context.Background(), node) + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + if log.calls.Load() != 1 { + t.Fatal("late notification handler admitted") + } +} diff --git a/pair.go b/pair.go index 0968ea599..5b8d1ccee 100644 --- a/pair.go +++ b/pair.go @@ -143,7 +143,7 @@ func (cli *Client) handlePairSuccess(ctx context.Context, node *waBinary.Node) { cli.dispatchEvent(&events.PairError{ID: jid, LID: lid, BusinessName: businessName, Platform: platform, Props: &props, Error: err}) } else { cli.Log.Infof("Successfully paired %s", cli.Store.ID) - go cli.sendUnifiedSession() + cli.startUnifiedSession() cli.dispatchEvent(&events.PairSuccess{ID: jid, LID: lid, BusinessName: businessName, Platform: platform, Props: &props}) } }() diff --git a/presence.go b/presence.go index eb378620a..7e39c6105 100644 --- a/presence.go +++ b/presence.go @@ -68,7 +68,7 @@ func (cli *Client) SendPresence(ctx context.Context, state types.Presence) error return ErrNoPushName } if state == types.PresenceAvailable { - go cli.sendUnifiedSession() + cli.startUnifiedSession() cli.sendActiveReceipts.CompareAndSwap(0, 1) } else { cli.sendActiveReceipts.CompareAndSwap(1, 0) diff --git a/send.go b/send.go index 48eafd8f9..9d756baf7 100644 --- a/send.go +++ b/send.go @@ -900,7 +900,7 @@ func (cli *Client) sendDM( storageJID := cli.resolveTCTokenStorageLID(ctx, to) if shouldSendTCTokenInChatAction(to) && shouldSendNewTCToken(cli.getTCTokenSenderTS(storageJID)) { - go cli.issuePrivacyTokenAndSave(storageJID, time.Now()) + cli.startPrivacyTokenIssue(storageJID, time.Now()) } return phash, data, nil diff --git a/tctoken.go b/tctoken.go index 2a812a11c..5f1bc4adc 100644 --- a/tctoken.go +++ b/tctoken.go @@ -146,21 +146,33 @@ func (cli *Client) deleteExpiredPrivacyTokens() { cli.tcTokenDBPruneLock.Unlock() return } - cli.lastTCTokenDBPrune = time.Now() - go func() { + pruneAt := time.Now() + if !cli.startConnectionTask(cli.BackgroundEventCtx, func(ctx context.Context) { defer cli.tcTokenDBPruneLock.Unlock() - deleted, err := cli.Store.PrivacyTokens.DeleteExpiredPrivacyTokens(cli.BackgroundEventCtx, currentTCTokenCutoffTimestamp()) + cli.lastTCTokenDBPrune = pruneAt + deleted, err := cli.Store.PrivacyTokens.DeleteExpiredPrivacyTokens(ctx, currentTCTokenCutoffTimestamp()) if err != nil { cli.Log.Warnf("Failed to remove expired tctokens from DB: %v", err) } else if deleted > 0 { cli.Log.Debugf("Removed %d expired tctokens from DB", deleted) } - }() + }) { + cli.tcTokenDBPruneLock.Unlock() + } } // Only called when a bucket boundary has been crossed since the last issuance. +func (cli *Client) startPrivacyTokenIssue(jid types.JID, senderTimestamp time.Time) bool { + return cli.startConnectionTask(cli.BackgroundEventCtx, func(ctx context.Context) { + cli.issuePrivacyTokenAndSaveContext(ctx, jid, senderTimestamp) + }) +} + func (cli *Client) issuePrivacyTokenAndSave(jid types.JID, senderTimestamp time.Time) { - ctx := cli.BackgroundEventCtx + cli.issuePrivacyTokenAndSaveContext(cli.BackgroundEventCtx, jid, senderTimestamp) +} + +func (cli *Client) issuePrivacyTokenAndSaveContext(ctx context.Context, jid types.JID, senderTimestamp time.Time) { storageJID := jid.ToNonAD() _, err := cli.issuePrivacyToken(ctx, storageJID, senderTimestamp) if err != nil { From 78366c622300c4c95ae59fc4b0f5f988d2bb9d45 Mon Sep 17 00:00:00 2001 From: Rajeh Taher Date: Thu, 22 Oct 2026 08:03:35 +0300 Subject: [PATCH 06/10] Own message history acknowledgement and retry task lifetimes --- CLIENT_LIFETIME.md | 56 +++- client.go | 6 +- message.go | 67 +++-- message_lifecycle.go | 152 +++++++++++ message_lifecycle_test.go | 555 ++++++++++++++++++++++++++++++++++++++ receipt.go | 8 +- retry.go | 20 +- 7 files changed, 814 insertions(+), 50 deletions(-) create mode 100644 message_lifecycle.go create mode 100644 message_lifecycle_test.go diff --git a/CLIENT_LIFETIME.md b/CLIENT_LIFETIME.md index 2d8192e01..1f065e695 100644 --- a/CLIENT_LIFETIME.md +++ b/CLIENT_LIFETIME.md @@ -42,6 +42,9 @@ the deadline ends its waiter and does not manufacture completion. - Async notification dispatches, server AB-props tasks and their ACK callback, privacy-token issuance/persistence, unified-session children and expired-token pruning, including the connect-success prune. See the exact boundaries below. +- Client-owned message name/event/history/storage/receipt/key-share launches, + ACK/retry handlers and delayed phone rerequests. Their exact contexts and loop + ownership are described below; appstate descendants remain separate gaps. - One teardown task, whose completion is recorded before launch. Context forwarding uses context.AfterFunc only to cancel contexts, with no user/protocol callbacks; those small cancellation callbacks are not a joined product-worker claim. @@ -140,6 +143,57 @@ External RefreshCAT implementations, store/transport internals and user-spawned callbacks do not gain whole-client completion proof from these wrappers. The established-context retention described above remains conservative and unchanged. +## Message, history, ACK and retry task ownership + +All explicit async launches in message.go, receipt.go and retry.go register +before scheduling. Typed helpers capture the original direct go-call arguments +before launch, including contact-name strings/JIDs, retry booleans, node/key-share +pointers and the identity event with its time.Now timestamp. The existing +anonymous history/peer receipt closures still read info.ID when executed. +Handler, crypto/key/store bodies and payloads are unchanged apart from the +inventoried scheduling/context/ownership controls. + +Name tasks and ACK/retry handlers keep caller contexts and values. Retry sends, +async history storage and event-buffer cleanup keep WithoutCancel ancestry, +then add final cancellation independently. History loops and delayed phone work +keep BackgroundEventCtx ancestry. Every admitted task stays counted until its +actual store, callback, send or handler returns, even if it ignores cancellation. +Ordinary Disconnect does not seal or cancel these tasks. + +Synchronous ACK/name/storage branches retain their original inline execution +and contexts, including canceled flags, NACKs and error policy. They are part of +an already admitted enclosing operation or separate caller-owned work. The +contextless DangerousInternals async-function adapter joins the supplied function +but cannot cancel an unknown captured context or own user-spawned descendants. + +History loop claims have exact nonzero-size owner tokens. Only that token can +release its claim or started indicator. Admission refusal releases its claim; +normal return/panic cleanup can restart queued work through a fresh admitted +owner. Restart logging runs inside the registered loop with cleanup installed. +The synchronous DangerousInternals loop adapter now takes the same exclusive +claim and joins its invocation; it cannot duplicate or clear another running +owner. This adapter keeps its signature and Background ancestry. Its other +caller-owned operations remain separate boundaries. + +A blocked history enqueue keeps ordinary backpressure and ignores ordinary +caller cancellation as before. Permanent final close interrupts it without +closing the queue. The bounded queue can retain pending items on final close; +no new loop or receipt is admitted to process them. The download/event/media +branch and one-minute idle timeout are unchanged. A loop stops on final close +and never restarts after that seal, while its actual in-flight work remains joined. + +Event-buffer cleanup retains the twelve-hour interval. A short mutex protects +its launch guard and timestamp, never a completion wait. Only admitted work +records the original captured time. Refusal releases the guard without advancing +that timestamp; actual store return releases it on every exit. + +Delayed phone rerequests keep their five-second delay, feature checks, per-ID +coalescing, cancellation and retry policy. The cancel map stores exact owner +tokens. Cleanup removes only that owner even after cancellation/replacement; +canceling an entry is not its completion. Refused launch creates no entry. A +synchronous DangerousInternals delayed call retains its Background behavior and +is still caller-owned. No timers/retry counts/protocol algorithms changed. + ## Still unbound The following existing client-owned producers retain their prior behavior and @@ -151,10 +205,8 @@ owning-library work before any successful stop result is possible. | Source at base 29a42459 | Unbound lifetime | | --- | --- | -| message.go:82-89,373-374,469-470,520,549,770,887,1005-1037 | Name updates, retry/ack work, event-buffer cleanup, event-dispatch wrapper, history download/storage/notification loops, key-share and peer-data work. | | appstate.go:162,645,659 | Missing-key request and async event batches. | | pair.go:138-147 | Outer pair completion and PrePairCallback. Its unified-session child is now tracked; the parent and its other boundaries are not. | -| receipt.go:27,36,109,128; retry.go:469,541 | Async ack/retry handlers and delayed phone rerequests. Cancellation of delayed requests is not their join. | | handshake.go:98-103 | GetClientPayload callback admission is not separately sealed; an admitted connection can reach it. The enclosing connect is counted, which is not full callback-admission coverage. | Caller-invoked public API methods, DangerousInternals, external HTTP transports, diff --git a/client.go b/client.go index a8cc1ad59..d4addbb96 100644 --- a/client.go +++ b/client.go @@ -120,6 +120,8 @@ type Client struct { EnableDecryptedEventBuffer bool synchronousMessageNameUpdates atomic.Bool lastDecryptedBufferClear time.Time + decryptedBufferClearMu sync.Mutex + decryptedBufferClearStarted bool DisableLoginAutoReconnect bool @@ -133,7 +135,7 @@ type Client struct { AppStateDebugLogs bool AutomaticMessageRerequestFromPhone bool - pendingPhoneRerequests map[types.MessageID]context.CancelFunc + pendingPhoneRerequests map[types.MessageID]*pendingPhoneRerequest pendingPhoneRerequestsLock sync.RWMutex appStateProc *appstate.Processor @@ -141,6 +143,8 @@ type Client struct { historySyncNotifications chan historySyncNotification historySyncHandlerStarted atomic.Bool + historySyncHandlerMu sync.Mutex + historySyncHandlerOwner *historySyncLoopOwner ManualHistorySyncDownload bool DisableManualHistorySyncReceipt bool DisableHistorySyncReceipt bool diff --git a/message.go b/message.go index c9a95ced3..78043afa0 100644 --- a/message.go +++ b/message.go @@ -79,14 +79,14 @@ func (cli *Client) updateMessageContactNames(ctx context.Context, info *types.Me if synchronous { cli.updateBusinessName(ctx, info.Sender, info.SenderAlt, info, verifiedName) } else { - go cli.updateBusinessName(ctx, info.Sender, info.SenderAlt, info, verifiedName) + cli.startMessageNameUpdate(ctx, info.Sender, info.SenderAlt, info, verifiedName, true) } } if len(info.PushName) > 0 && info.PushName != "-" && (cli.MessengerConfig == nil || info.PushName != "username") { if synchronous { cli.updatePushName(ctx, info.Sender, info.SenderAlt, info, info.PushName) } else { - go cli.updatePushName(ctx, info.Sender, info.SenderAlt, info, info.PushName) + cli.startMessageNameUpdate(ctx, info.Sender, info.SenderAlt, info, info.PushName, false) } } } @@ -348,7 +348,7 @@ func (cli *Client) decryptMessages(ctx context.Context, info *types.MessageInfo, if ok && len(node.GetChildrenByTag("enc")) == 0 { uType := events.UnavailableType(unavailableNode.AttrGetter().String("type")) cli.Log.Warnf("Unavailable message %s from %s (type: %q)", info.ID, info.SourceString(), uType) - cli.backgroundIfAsyncAck(func() { + cli.backgroundIfAsyncAckContext(ctx, func(ctx context.Context) { cli.immediateRequestMessageFromPhone(ctx, info) cli.sendAck(ctx, node, 0) }) @@ -370,8 +370,8 @@ func (cli *Client) decryptMessages(ctx context.Context, info *types.MessageInfo, cli.sendRetryReceipt(ctx, node, info, false) cli.sendAck(ctx, node, 0) } else { - go cli.sendRetryReceipt(context.WithoutCancel(ctx), node, info, false) - go cli.sendAck(ctx, node, 0) + cli.startRetryReceipt(context.WithoutCancel(ctx), node, info, false) + cli.startAck(ctx, node, 0) } cli.dispatchEvent(&events.UndecryptableMessage{Info: *info}) return @@ -442,7 +442,7 @@ func (cli *Client) decryptMessages(ctx context.Context, info *types.MessageInfo, // receipt would only provoke redeliveries. Acknowledge, surface the // message as undecryptable, and move on. cli.Log.Warnf("Ignoring group message %s from %s: %v", info.ID, info.SourceString(), err) - cli.backgroundIfAsyncAck(func() { + cli.backgroundIfAsyncAckContext(ctx, func(ctx context.Context) { cli.sendAck(ctx, node, 0) }) cli.dispatchEvent(&events.UndecryptableMessage{ @@ -458,7 +458,7 @@ func (cli *Client) decryptMessages(ctx context.Context, info *types.MessageInfo, } isUnavailable := encType == "skmsg" && !containsDirectMsg && errors.Is(err, signalerror.ErrNoSenderKeyForUser) if encType == "msmsg" { - cli.backgroundIfAsyncAck(func() { + cli.backgroundIfAsyncAckContext(ctx, func(ctx context.Context) { cli.sendAck(ctx, node, NackMissingMessageSecret) }) } else if cli.SynchronousAck { @@ -466,8 +466,8 @@ func (cli *Client) decryptMessages(ctx context.Context, info *types.MessageInfo, // TODO this probably isn't supposed to ack cli.sendAck(ctx, node, 0) } else { - go cli.sendRetryReceipt(context.WithoutCancel(ctx), node, info, isUnavailable) - go cli.sendAck(ctx, node, 0) + cli.startRetryReceipt(context.WithoutCancel(ctx), node, info, isUnavailable) + cli.startAck(ctx, node, 0) } cli.dispatchEvent(&events.UndecryptableMessage{ Info: *info, @@ -515,18 +515,10 @@ func (cli *Client) decryptMessages(ctx context.Context, info *types.MessageInfo, Msg("Deleted event plaintext from buffer") } - if time.Since(cli.lastDecryptedBufferClear) > 12*time.Hour && ctx.Err() == nil { - cli.lastDecryptedBufferClear = time.Now() - go func() { - err := cli.Store.EventBuffer.DeleteOldBufferedHashes(context.WithoutCancel(ctx)) - if err != nil { - zerolog.Ctx(ctx).Err(err).Msg("Failed to delete old buffered hashes") - } - }() - } + cli.startEventBufferCleanup(ctx) } } - cli.backgroundIfAsyncAck(func() { + cli.backgroundIfAsyncAckContext(ctx, func(ctx context.Context) { if !recognizedStanza { cli.sendAck(ctx, node, NackUnrecognizedStanza) } else if protobufFailed { @@ -546,7 +538,7 @@ func (cli *Client) clearUntrustedIdentity(ctx context.Context, target types.JID) if err != nil { return fmt.Errorf("failed to delete session: %w", err) } - go cli.dispatchEvent(&events.IdentityChange{JID: target, Timestamp: time.Now(), Implicit: true}) + cli.dispatchEventAsync(&events.IdentityChange{JID: target, Timestamp: time.Now(), Implicit: true}) return nil } @@ -755,9 +747,9 @@ func (cli *Client) handleSenderKeyDistributionMessage(ctx context.Context, chat, cli.Log.Debugf("Processed sender key distribution message from %s in %s", senderKeyName.Sender().String(), senderKeyName.GroupID()) } -func (cli *Client) handleHistorySyncNotificationLoop() { +func (cli *Client) runHistorySyncNotificationLoop(ctx context.Context, owner *historySyncLoopOwner, restart bool) { defer func() { - cli.historySyncHandlerStarted.Store(false) + released := cli.releaseHistorySyncLoop(owner) err := recover() if err != nil { cli.Log.Errorf("History sync handler panicked: %v\n%s", err, debug.Stack()) @@ -765,14 +757,20 @@ func (cli *Client) handleHistorySyncNotificationLoop() { // Check in case something new appeared in the channel between the loop stopping // and the atomic variable being updated. If yes, restart the loop. - if len(cli.historySyncNotifications) > 0 && cli.historySyncHandlerStarted.CompareAndSwap(false, true) { - cli.Log.Warnf("New history sync notifications appeared after loop stopped, restarting loop...") - go cli.handleHistorySyncNotificationLoop() + if released && !cli.nativeClosed() && len(cli.historySyncNotifications) > 0 { + cli.startHistorySyncLoop(true) } }() - ctx := cli.BackgroundEventCtx + if restart { + cli.Log.Warnf("New history sync notifications appeared after loop stopped, restarting loop...") + } for { + if cli.nativeClosed() { + return + } select { + case <-cli.nativeDone(): + return case queued := <-cli.historySyncNotifications: blob, err := cli.DownloadHistorySync(ctx, queued.notification, false) if err != nil { @@ -884,7 +882,7 @@ func (cli *Client) DownloadHistorySync(ctx context.Context, notif *waE2E.History } else if synchronousStorage { doStorage(storageCtx) } else { - go doStorage(storageCtx) + cli.startConnectionTask(storageCtx, doStorage) } return &historySync, nil } @@ -1000,18 +998,17 @@ func (cli *Client) handleProtocolMessage(ctx context.Context, info *types.Messag if protoMsg.GetHistorySyncNotification() != nil { if !cli.ManualHistorySyncDownload { - cli.historySyncNotifications <- historySyncNotification{messageID: info.ID, notification: protoMsg.HistorySyncNotification} - if cli.historySyncHandlerStarted.CompareAndSwap(false, true) { - go cli.handleHistorySyncNotificationLoop() + if !cli.enqueueHistorySyncNotification(historySyncNotification{messageID: info.ID, notification: protoMsg.HistorySyncNotification}) { + return false } } if cli.shouldSendHistorySyncReceipt() { - go func() { + cli.startConnectionTask(ctx, func(ctx context.Context) { err := cli.SendProtocolMessageReceipt(ctx, info.ID, types.ReceiptTypeHistorySync) if err != nil { cli.Log.Warnf("Failed to send acknowledgement for protocol message %s: %v", info.ID, err) } - }() + }) } } @@ -1030,16 +1027,16 @@ func (cli *Client) handleProtocolMessage(ctx context.Context, info *types.Messag } if protoMsg.GetAppStateSyncKeyShare() != nil { - go cli.handleAppStateSyncKeyShare(context.WithoutCancel(ctx), protoMsg.AppStateSyncKeyShare) + cli.startAppStateKeyShare(context.WithoutCancel(ctx), protoMsg.AppStateSyncKeyShare) } if info.Category == "peer" { - go func() { + cli.startConnectionTask(ctx, func(ctx context.Context) { err := cli.SendProtocolMessageReceipt(ctx, info.ID, types.ReceiptTypePeerMsg) if err != nil { cli.Log.Warnf("Failed to send acknowledgement for protocol message %s: %v", info.ID, err) } - }() + }) } return } diff --git a/message_lifecycle.go b/message_lifecycle.go new file mode 100644 index 000000000..313130130 --- /dev/null +++ b/message_lifecycle.go @@ -0,0 +1,152 @@ +package whatsmeow + +import ( + "context" + "time" + + waBinary "github.com/polymorfa/hypermeow/binary" + "github.com/polymorfa/hypermeow/proto/waE2E" + "github.com/polymorfa/hypermeow/types" + "github.com/polymorfa/hypermeow/types/events" + "github.com/rs/zerolog" +) + +// Typed parameters retain the original go-call argument evaluation before launch. +func (cli *Client) startMessageNameUpdate(ctx context.Context, user, alt types.JID, info *types.MessageInfo, name string, business bool) bool { + return cli.startConnectionTask(ctx, func(ctx context.Context) { + if business { + cli.updateBusinessName(ctx, user, alt, info, name) + } else { + cli.updatePushName(ctx, user, alt, info, name) + } + }) +} +func (cli *Client) startAck(ctx context.Context, node *waBinary.Node, code int) bool { + return cli.startConnectionTask(ctx, func(ctx context.Context) { cli.sendAck(ctx, node, code) }) +} +func (cli *Client) startRetryReceipt(ctx context.Context, node *waBinary.Node, info *types.MessageInfo, force bool) bool { + return cli.startConnectionTask(ctx, func(ctx context.Context) { cli.sendRetryReceipt(ctx, node, info, force) }) +} +func (cli *Client) startRetryHandler(ctx context.Context, receipt *events.Receipt, node *waBinary.Node) bool { + return cli.startConnectionTask(ctx, func(ctx context.Context) { cli.tryHandleRetryReceipt(ctx, receipt, node) }) +} +func (cli *Client) startAppStateKeyShare(ctx context.Context, keys *waE2E.AppStateSyncKeyShare) bool { + return cli.startConnectionTask(ctx, func(ctx context.Context) { cli.handleAppStateSyncKeyShare(ctx, keys) }) +} +func (cli *Client) backgroundIfAsyncAckContext(ctx context.Context, fn func(context.Context)) { + if cli.SynchronousAck { + fn(ctx) + } else { + cli.startConnectionTask(ctx, fn) + } +} + +func (cli *Client) startEventBufferCleanup(ctx context.Context) bool { + cli.decryptedBufferClearMu.Lock() + if cli.decryptedBufferClearStarted || time.Since(cli.lastDecryptedBufferClear) <= 12*time.Hour || ctx.Err() != nil { + cli.decryptedBufferClearMu.Unlock() + return false + } + cli.decryptedBufferClearStarted = true + clearAt := time.Now() + cli.decryptedBufferClearMu.Unlock() + if !cli.startConnectionTask(context.WithoutCancel(ctx), func(taskCtx context.Context) { + defer func() { + cli.decryptedBufferClearMu.Lock() + cli.decryptedBufferClearStarted = false + cli.decryptedBufferClearMu.Unlock() + }() + cli.decryptedBufferClearMu.Lock() + cli.lastDecryptedBufferClear = clearAt + cli.decryptedBufferClearMu.Unlock() + err := cli.Store.EventBuffer.DeleteOldBufferedHashes(taskCtx) + if err != nil { + zerolog.Ctx(ctx).Err(err).Msg("Failed to delete old buffered hashes") + } + }) { + cli.decryptedBufferClearMu.Lock() + cli.decryptedBufferClearStarted = false + cli.decryptedBufferClearMu.Unlock() + return false + } + return true +} + +type historySyncLoopOwner struct{ reserved bool } + +func (cli *Client) claimHistorySyncLoop() *historySyncLoopOwner { + cli.historySyncHandlerMu.Lock() + defer cli.historySyncHandlerMu.Unlock() + if cli.historySyncHandlerOwner != nil { + return nil + } + owner := &historySyncLoopOwner{reserved: true} + cli.historySyncHandlerOwner = owner + cli.historySyncHandlerStarted.Store(true) + return owner +} +func (cli *Client) releaseHistorySyncLoop(owner *historySyncLoopOwner) bool { + cli.historySyncHandlerMu.Lock() + defer cli.historySyncHandlerMu.Unlock() + if owner == nil || cli.historySyncHandlerOwner != owner { + return false + } + cli.historySyncHandlerOwner = nil + cli.historySyncHandlerStarted.Store(false) + return true +} +func (cli *Client) startHistorySyncLoop(restart bool) bool { + owner := cli.claimHistorySyncLoop() + if owner == nil { + return false + } + if !cli.startConnectionTask(cli.BackgroundEventCtx, func(ctx context.Context) { cli.runHistorySyncNotificationLoop(ctx, owner, restart) }) { + cli.releaseHistorySyncLoop(owner) + return false + } + return true +} + +// The synchronous DangerousInternals adapter retains its signature and takes its +// own exact claim; it cannot clear or duplicate a different running loop owner. +func (cli *Client) handleHistorySyncNotificationLoop() { + owner := cli.claimHistorySyncLoop() + if owner == nil { + return + } + done, ok := cli.enterNative() + if !ok { + cli.releaseHistorySyncLoop(owner) + return + } + defer done() + ctx, release := cli.nativeContext(cli.BackgroundEventCtx) + defer release() + cli.runHistorySyncNotificationLoop(ctx, owner, false) +} +func (cli *Client) enqueueHistorySyncNotification(queued historySyncNotification) bool { + if cli.nativeClosed() { + return false + } + select { + case <-cli.nativeDone(): + return false + case cli.historySyncNotifications <- queued: + } + if cli.nativeClosed() { + return false + } + cli.startHistorySyncLoop(false) + return !cli.nativeClosed() +} + +type pendingPhoneRerequest struct{ cancel context.CancelFunc } + +func (cli *Client) startDelayedRequestMessageFromPhone(info *types.MessageInfo) bool { + return cli.startConnectionTask(cli.BackgroundEventCtx, func(ctx context.Context) { cli.delayedRequestMessageFromPhoneContext(ctx, info) }) +} + +// Caller-owned synchronous adapter keeps the original Background ancestry. +func (cli *Client) delayedRequestMessageFromPhone(info *types.MessageInfo) { + cli.delayedRequestMessageFromPhoneContext(cli.BackgroundEventCtx, info) +} diff --git a/message_lifecycle_test.go b/message_lifecycle_test.go new file mode 100644 index 000000000..dfa8f6056 --- /dev/null +++ b/message_lifecycle_test.go @@ -0,0 +1,555 @@ +package whatsmeow + +import ( + "bytes" + "compress/zlib" + "context" + "errors" + "github.com/polymorfa/hypermeow/proto/waE2E" + "github.com/polymorfa/hypermeow/proto/waHistorySync" + waLog "github.com/polymorfa/hypermeow/util/log" + "google.golang.org/protobuf/proto" + "sync" + "sync/atomic" + "testing" + "time" + + waBinary "github.com/polymorfa/hypermeow/binary" + "github.com/polymorfa/hypermeow/store" + "github.com/polymorfa/hypermeow/types" +) + +type finalMessageRelay struct { + *fakeShadowRelay + entered chan context.Context + release chan struct{} +} + +func (r *finalMessageRelay) SendNode(ctx context.Context, _ []byte) error { + r.entered <- ctx + <-r.release + return ctx.Err() +} +func TestFinalMessageAckJoinsActualSend(t *testing.T) { + c := NewClient(&store.Device{}, nil) + relay := &finalMessageRelay{fakeShadowRelay: &fakeShadowRelay{}, entered: make(chan context.Context, 1), release: make(chan struct{})} + c.shadowRelay = relay + var once sync.Once + release := func() { once.Do(func() { close(relay.release) }) } + t.Cleanup(func() { + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + parent := context.WithValue(context.Background(), finalConnectionContextKey{}, "ack-value") + c.maybeDeferredAck(parent, &waBinary.Node{Tag: "message", Attrs: waBinary.Attrs{"id": "synthetic", "from": types.ServerJID}}) + var ctx context.Context + select { + case ctx = <-relay.entered: + case <-time.After(2 * time.Second): + t.Fatal("ACK not reached") + } + if ctx.Value(finalConnectionContextKey{}) != "ack-value" { + t.Fatal("ACK value lost") + } + c.Disconnect() + if ctx.Err() != nil { + t.Fatal("ordinary disconnect cancelled ACK") + } + finalTestBlocked(t, c) + finalTestWait(t, ctx.Done()) + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } +} + +type finalMessageStoreCall struct { + ctx context.Context + user types.JID + name string +} +type finalMessageStore struct { + store.NoopStore + entered chan finalMessageStoreCall + release chan struct{} +} + +func (s *finalMessageStore) PutPushName(ctx context.Context, user types.JID, name string) (bool, string, error) { + s.entered <- finalMessageStoreCall{ctx, user, name} + <-s.release + return false, "", nil +} +func (s *finalMessageStore) DeleteOldBufferedHashes(ctx context.Context) error { + s.entered <- finalMessageStoreCall{ctx: ctx} + <-s.release + return nil +} +func finalMessageFixture(t *testing.T) (*Client, *finalMessageStore, func()) { + t.Helper() + s := &finalMessageStore{entered: make(chan finalMessageStoreCall, 2), release: make(chan struct{})} + c := NewClient(&store.Device{Contacts: s, EventBuffer: s}, nil) + var once sync.Once + release := func() { once.Do(func() { close(s.release) }) } + t.Cleanup(func() { + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + return c, s, release +} +func finalMessageStoreEntered(t *testing.T, s *finalMessageStore) finalMessageStoreCall { + t.Helper() + select { + case got := <-s.entered: + return got + case <-time.After(2 * time.Second): + t.Fatal("store not reached") + return finalMessageStoreCall{} + } +} + +type finalMessageParent struct { + context.Context + entered, release chan struct{} + once sync.Once +} + +func (p *finalMessageParent) Value(key any) any { + p.once.Do(func() { close(p.entered); <-p.release }) + return p.Context.Value(key) +} +func TestFinalMessageNameArgumentsAndActualStore(t *testing.T) { + c, s, release := finalMessageFixture(t) + base, cancel := context.WithCancel(context.Background()) + defer cancel() + parent := &finalMessageParent{Context: base, entered: make(chan struct{}), release: make(chan struct{})} + var once sync.Once + unblock := func() { once.Do(func() { close(parent.release) }) } + // This cleanup runs before fixture teardown, including assertion failures. + launched := make(chan struct{}) + t.Cleanup(func() { unblock(); finalTestWait(t, launched) }) + info := &types.MessageInfo{MessageSource: types.MessageSource{Sender: types.NewJID("old", types.DefaultUserServer)}, PushName: "old-name"} + go func() { c.updateMessageContactNames(parent, info); close(launched) }() + finalTestWait(t, parent.entered) // Context construction precedes goroutine scheduling. + info.Sender = types.NewJID("new", types.DefaultUserServer) + info.PushName = "new-name" + unblock() + finalTestWait(t, launched) + got := finalMessageStoreEntered(t, s) + if got.user.User != "old" || got.name != "old-name" { + t.Fatalf("go-call values moved into worker: %v %q", got.user, got.name) + } + finalTestBlocked(t, c) + finalTestWait(t, got.ctx.Done()) + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } +} +func TestFinalMessageBufferWithoutCancelAndRefusal(t *testing.T) { + c, s, release := finalMessageFixture(t) + parent, cancel := context.WithCancel(context.WithValue(context.Background(), finalConnectionContextKey{}, "buffer-value")) + defer cancel() + if !c.startEventBufferCleanup(parent) { + t.Fatal("cleanup refused") + } + got := finalMessageStoreEntered(t, s) + cancel() + c.Disconnect() + if got.ctx.Err() != nil || got.ctx.Value(finalConnectionContextKey{}) != "buffer-value" { + t.Fatal("WithoutCancel semantics lost") + } + if c.startEventBufferCleanup(context.Background()) { + t.Fatal("cleanup not coalesced") + } + finalTestBlocked(t, c) + finalTestWait(t, got.ctx.Done()) + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + c.decryptedBufferClearMu.Lock() + busy := c.decryptedBufferClearStarted + c.decryptedBufferClearMu.Unlock() + if busy { + t.Fatal("cleanup guard retained") + } + retired, _, _ := finalMessageFixture(t) + if err := retired.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + if retired.startEventBufferCleanup(context.Background()) { + t.Fatal("post-seal cleanup admitted") + } + retired.decryptedBufferClearMu.Lock() + defer retired.decryptedBufferClearMu.Unlock() + if retired.decryptedBufferClearStarted || !retired.lastDecryptedBufferClear.IsZero() { + t.Fatal("refusal stranded guard or advanced timestamp") + } +} + +func finalMessageHistoryPayload(t *testing.T) *waE2E.HistorySyncNotification { + t.Helper() + raw, err := proto.Marshal(&waHistorySync.HistorySync{SyncType: waHistorySync.HistorySync_PUSH_NAME.Enum(), Pushnames: []*waHistorySync.Pushname{{ID: proto.String("synthetic@s.whatsapp.net"), Pushname: proto.String("synthetic")}}}) + if err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + w := zlib.NewWriter(&buf) + if _, err = w.Write(raw); err != nil { + t.Fatal(err) + } + if err = w.Close(); err != nil { + t.Fatal(err) + } + return &waE2E.HistorySyncNotification{InitialHistBootstrapInlinePayload: buf.Bytes()} +} +func TestFinalMessageHistoryStorageContexts(t *testing.T) { + for _, synchronous := range []bool{false, true} { + t.Run(map[bool]string{false: "async", true: "sync"}[synchronous], func(t *testing.T) { + c, s, release := finalMessageFixture(t) + parent, cancel := context.WithCancel(context.WithValue(context.Background(), finalConnectionContextKey{}, "history-value")) + defer cancel() + returned := make(chan error, 1) + exited := make(chan struct{}) + t.Cleanup(func() { release(); finalTestWait(t, exited) }) + notif := finalMessageHistoryPayload(t) + go func() { + defer close(exited) + _, err := c.DownloadHistorySync(parent, notif, synchronous) + returned <- err + }() + got := finalMessageStoreEntered(t, s) + if got.ctx.Value(finalConnectionContextKey{}) != "history-value" { + t.Fatal("history value lost") + } + cancel() + c.Disconnect() + if synchronous { + if got.ctx != parent || !errors.Is(got.ctx.Err(), context.Canceled) { + t.Fatal("synchronous storage context changed") + } + select { + case <-returned: + t.Fatal("synchronous storage returned early") + default: + } + release() + } else { + if got.ctx.Err() != nil { + t.Fatal("async storage lost WithoutCancel") + } + select { + case err := <-returned: + if err != nil { + t.Fatal(err) + } + case <-time.After(2 * time.Second): + t.Fatal("async download did not return") + } + finalTestBlocked(t, c) + finalTestWait(t, got.ctx.Done()) + release() + } + if synchronous { + select { + case err := <-returned: + if err != nil { + t.Fatal(err) + } + case <-time.After(2 * time.Second): + t.Fatal("sync download stuck") + } + } + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + }) + } +} + +type finalMessageHistoryLog struct { + waLog.Logger + first, second, releaseFirst, releaseSecond chan struct{} + calls atomic.Int32 + restarted chan struct{} +} + +func (l *finalMessageHistoryLog) Sub(string) waLog.Logger { return l } +func (l *finalMessageHistoryLog) Errorf(msg string, args ...any) { + if msg == "Failed to download history sync: %v" { + if l.calls.Add(1) == 1 { + close(l.first) + <-l.releaseFirst + panic("synthetic loop restart") + } else { + close(l.second) + <-l.releaseSecond + } + } +} +func (l *finalMessageHistoryLog) Warnf(msg string, args ...any) { + if msg == "New history sync notifications appeared after loop stopped, restarting loop..." { + close(l.restarted) + } +} +func TestFinalMessageHistoryRestartAndExactClaim(t *testing.T) { + log := &finalMessageHistoryLog{Logger: waLog.Noop, first: make(chan struct{}), second: make(chan struct{}), releaseFirst: make(chan struct{}), releaseSecond: make(chan struct{}), restarted: make(chan struct{})} + c := NewClient(&store.Device{}, log) + var once1, once2 sync.Once + release1 := func() { once1.Do(func() { close(log.releaseFirst) }) } + release2 := func() { once2.Do(func() { close(log.releaseSecond) }) } + t.Cleanup(func() { + release1() + release2() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + bad := historySyncNotification{notification: &waE2E.HistorySyncNotification{InitialHistBootstrapInlinePayload: []byte("not zlib")}} + if !c.enqueueHistorySyncNotification(bad) { + t.Fatal("queue refused") + } + finalTestWait(t, log.first) + c.historySyncHandlerMu.Lock() + old := c.historySyncHandlerOwner + c.historySyncHandlerMu.Unlock() + if !c.enqueueHistorySyncNotification(bad) { + t.Fatal("queue refused while loop running") + } + c.Disconnect() + release1() + finalTestWait(t, log.restarted) + finalTestWait(t, log.second) + if c.releaseHistorySyncLoop(old) { + t.Fatal("old owner cleared replacement") + } + if !c.historySyncHandlerStarted.Load() { + t.Fatal("replacement claim cleared") + } + finalTestBlocked(t, c) + release2() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + if c.historySyncHandlerStarted.Load() { + t.Fatal("loop claim stranded") + } + if c.startHistorySyncLoop(false) || c.historySyncHandlerStarted.Load() { + t.Fatal("refused loop retained claim") + } + if c.enqueueHistorySyncNotification(bad) { + t.Fatal("post-seal queue accepted") + } +} +func TestFinalMessageHistoryBlockedEnqueueCancels(t *testing.T) { + c := NewClient(&store.Device{}, nil) + // An unbuffered queue with no consumer exercises the original backpressure. + c.historySyncNotifications = make(chan historySyncNotification) + done, ok := c.enterNative() + if !ok { + t.Fatal("fixture admission") + } + result := make(chan bool, 1) + ready := make(chan struct{}) + go func() { + defer done() + close(ready) + result <- c.enqueueHistorySyncNotification(historySyncNotification{}) + }() + t.Cleanup(func() { + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + finalTestWait(t, ready) + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + select { + case accepted := <-result: + if accepted { + t.Fatal("blocked enqueue accepted after seal") + } + case <-time.After(2 * time.Second): + t.Fatal("enqueue stranded") + } + if c.historySyncHandlerStarted.Load() { + t.Fatal("enqueue created late loop") + } +} + +type finalMessagePhoneLog struct { + waLog.Logger + entered, release chan struct{} +} + +func (l *finalMessagePhoneLog) Sub(string) waLog.Logger { return l } +func (l *finalMessagePhoneLog) Debugf(msg string, args ...any) { + if msg == "Cancelled delayed request for message %s from phone" { + close(l.entered) + <-l.release + } +} +func TestFinalMessageDelayedPhoneCancelJoinsAndProtectsReplacement(t *testing.T) { + log := &finalMessagePhoneLog{Logger: waLog.Noop, entered: make(chan struct{}), release: make(chan struct{})} + c := NewClient(&store.Device{}, log) + c.AutomaticMessageRerequestFromPhone = true + // Permanent cancellation reaches the real cancellation path without changing + // the original five-second policy timer or invoking crypto. + c.BackgroundEventCtx = context.Background() + var once sync.Once + release := func() { once.Do(func() { close(log.release) }) } + t.Cleanup(func() { + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + if !c.startDelayedRequestMessageFromPhone(&types.MessageInfo{ID: "synthetic"}) { + t.Fatal("delayed task refused") + } + c.Disconnect() + finalTestBlocked(t, c) + finalTestWait(t, log.entered) + c.pendingPhoneRerequestsLock.Lock() + old := c.pendingPhoneRerequests["synthetic"] + replacement := &pendingPhoneRerequest{cancel: func() {}} + c.pendingPhoneRerequests["synthetic"] = replacement + c.pendingPhoneRerequestsLock.Unlock() + if old == nil { + t.Fatal("cancel map entry absent") + } + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + c.pendingPhoneRerequestsLock.Lock() + got := c.pendingPhoneRerequests["synthetic"] + delete(c.pendingPhoneRerequests, "synthetic") + c.pendingPhoneRerequestsLock.Unlock() + if got != replacement { + t.Fatal("old cleanup cleared replacement") + } + if c.startDelayedRequestMessageFromPhone(&types.MessageInfo{ID: "late"}) { + t.Fatal("late delayed task admitted") + } + c.pendingPhoneRerequestsLock.RLock() + n := len(c.pendingPhoneRerequests) + c.pendingPhoneRerequestsLock.RUnlock() + if n != 0 { + t.Fatal("refused launch left cancel entry") + } +} +func TestFinalMessageSynchronousAckBranches(t *testing.T) { + c := NewClient(&store.Device{}, nil) + t.Cleanup(func() { + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + c.SynchronousAck = true + parent, cancel := context.WithCancel(context.Background()) + cancel() + called := false + c.backgroundIfAsyncAckContext(parent, func(ctx context.Context) { + if ctx != parent { + t.Error("synchronous context changed") + } + called = true + }) + if !called { + t.Fatal("synchronous callback delayed") + } + legacy := false + c.backgroundIfAsyncAck(func() { legacy = true }) + if !legacy { + t.Fatal("synchronous fn delayed") + } + relay := &finalMessageRelay{fakeShadowRelay: &fakeShadowRelay{}, entered: make(chan context.Context, 4), release: make(chan struct{})} + close(relay.release) // A regression must fail assertions rather than strand the fixture. + c.shadowRelay = relay + // Both synchronous cancellation paths must suppress ACK without reaching send. + node := &waBinary.Node{Tag: "message", Attrs: waBinary.Attrs{"id": "synthetic", "from": types.ServerJID}} + c.maybeDeferredAck(parent, node)() + cancelled := true + c.maybeDeferredAck(context.Background(), node)(&cancelled) + select { + case <-relay.entered: + t.Fatal("cancelled synchronous ACK sent") + default: + } + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + c.SynchronousAck = false + called = false + c.backgroundIfAsyncAck(func() { called = true }) + c.backgroundIfAsyncAckContext(context.Background(), func(context.Context) { called = true }) + if called { + t.Fatal("async fn admitted after seal") + } +} + +func TestFinalMessageDelayedPhoneOrdinaryCancelCleanup(t *testing.T) { + log := &finalMessagePhoneLog{Logger: waLog.Noop, entered: make(chan struct{}), release: make(chan struct{})} + c := NewClient(&store.Device{}, log) + c.AutomaticMessageRerequestFromPhone = true + parent, cancel := context.WithCancel(context.Background()) + cancel() + c.BackgroundEventCtx = parent + var once sync.Once + release := func() { once.Do(func() { close(log.release) }) } + t.Cleanup(func() { + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + if !c.startDelayedRequestMessageFromPhone(&types.MessageInfo{ID: "ordinary"}) { + t.Fatal("task refused") + } + finalTestWait(t, log.entered) + // Real pending-map cancel and clear operate on the exact registered owner. + c.cancelDelayedRequestFromPhone("ordinary") + c.clearDelayedMessageRequests() + // Call the same private body synchronously while the first owner is held, + // so this assertion cannot become a later fresh request during teardown. + c.delayedRequestMessageFromPhone(&types.MessageInfo{ID: "ordinary"}) + finalTestBlocked(t, c) + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + c.pendingPhoneRerequestsLock.RLock() + n := len(c.pendingPhoneRerequests) + c.pendingPhoneRerequestsLock.RUnlock() + if n != 0 { + t.Fatal("ordinary cancellation retained pending owner") + } + if finalContextWatcherCount(c) != 0 { + t.Fatal("task forwarding watcher retained") + } +} +func TestFinalMessageAsyncFunctionActualJoin(t *testing.T) { + c := NewClient(&store.Device{}, nil) + entered, release := make(chan struct{}), make(chan struct{}) + var once sync.Once + unblock := func() { once.Do(func() { close(release) }) } + t.Cleanup(func() { + unblock() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + c.backgroundIfAsyncAck(func() { close(entered); <-release }) + finalTestWait(t, entered) + finalTestBlocked(t, c) + unblock() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } +} diff --git a/receipt.go b/receipt.go index cc58b5f24..4102e3e39 100644 --- a/receipt.go +++ b/receipt.go @@ -24,7 +24,7 @@ func (cli *Client) handleReceipt(ctx context.Context, node *waBinary.Node) { receipt, participants, err := cli.parseReceipt(node) if err != nil { cli.Log.Warnf("Failed to parse receipt: %v", err) - go cli.sendAck(ctx, node, NackParsingError) + cli.startAck(ctx, node, NackParsingError) } else if participants != nil { defer cli.maybeDeferredAck(ctx, node)(&cancelled) for _, pcp := range participants { @@ -33,7 +33,7 @@ func (cli *Client) handleReceipt(ctx context.Context, node *waBinary.Node) { } else { if receipt.Type == types.ReceiptTypeRetry { // Ack happens inside the retry receipt handler - go cli.tryHandleRetryReceipt(ctx, receipt, node) + cli.startRetryHandler(ctx, receipt, node) } else { defer cli.maybeDeferredAck(ctx, node)(&cancelled) } @@ -106,7 +106,7 @@ func (cli *Client) backgroundIfAsyncAck(fn func()) { if cli.SynchronousAck { fn() } else { - go fn() + cli.startNative(fn) } } @@ -125,7 +125,7 @@ func (cli *Client) maybeDeferredAck(ctx context.Context, node *waBinary.Node) fu cli.sendAck(ctx, node, 0) } } else { - go cli.sendAck(ctx, node, 0) + cli.startAck(ctx, node, 0) return func(...*bool) {} } } diff --git a/retry.go b/retry.go index 91b612674..62d4adf4a 100644 --- a/retry.go +++ b/retry.go @@ -447,7 +447,7 @@ func (cli *Client) cancelDelayedRequestFromPhone(msgID types.MessageID) { cli.pendingPhoneRerequestsLock.RLock() cancelPendingRequest, ok := cli.pendingPhoneRerequests[msgID] if ok { - cancelPendingRequest() + cancelPendingRequest.cancel() } cli.pendingPhoneRerequestsLock.RUnlock() } @@ -456,7 +456,7 @@ func (cli *Client) cancelDelayedRequestFromPhone(msgID types.MessageID) { // This is only used if Client.AutomaticMessageRerequestFromPhone is true. var RequestFromPhoneDelay = 5 * time.Second -func (cli *Client) delayedRequestMessageFromPhone(info *types.MessageInfo) { +func (cli *Client) delayedRequestMessageFromPhoneContext(parent context.Context, info *types.MessageInfo) { if !cli.AutomaticMessageRerequestFromPhone || cli.MessengerConfig != nil { return } @@ -466,14 +466,18 @@ func (cli *Client) delayedRequestMessageFromPhone(info *types.MessageInfo) { cli.pendingPhoneRerequestsLock.Unlock() return } - ctx, cancel := context.WithCancel(cli.BackgroundEventCtx) + ctx, cancel := context.WithCancel(parent) defer cancel() - ensureMap(&cli.pendingPhoneRerequests)[info.ID] = cancel + owner := &pendingPhoneRerequest{cancel: cancel} + msgID := info.ID + ensureMap(&cli.pendingPhoneRerequests)[msgID] = owner cli.pendingPhoneRerequestsLock.Unlock() defer func() { cli.pendingPhoneRerequestsLock.Lock() - delete(cli.pendingPhoneRerequests, info.ID) + if cli.pendingPhoneRerequests[msgID] == owner { + delete(cli.pendingPhoneRerequests, msgID) + } cli.pendingPhoneRerequestsLock.Unlock() }() select { @@ -497,8 +501,8 @@ func (cli *Client) immediateRequestMessageFromPhone(ctx context.Context, info *t func (cli *Client) clearDelayedMessageRequests() { cli.pendingPhoneRerequestsLock.Lock() defer cli.pendingPhoneRerequestsLock.Unlock() - for _, cancel := range cli.pendingPhoneRerequests { - cancel() + for _, owner := range cli.pendingPhoneRerequests { + owner.cancel() } } @@ -538,7 +542,7 @@ func (cli *Client) sendRetryReceipt(ctx context.Context, node *waBinary.Node, in if cli.SynchronousAck { cli.immediateRequestMessageFromPhone(ctx, info) } else { - go cli.delayedRequestMessageFromPhone(info) + cli.startDelayedRequestMessageFromPhone(info) } } From efcf8fd9291ed7785dd703699ddac2cdf13c6170 Mon Sep 17 00:00:00 2001 From: Rajeh Taher Date: Thu, 22 Oct 2026 08:18:32 +0300 Subject: [PATCH 07/10] Clarify Disconnect cancellation of registered phone requests --- CLIENT_LIFETIME.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CLIENT_LIFETIME.md b/CLIENT_LIFETIME.md index 1f065e695..de8d68fab 100644 --- a/CLIENT_LIFETIME.md +++ b/CLIENT_LIFETIME.md @@ -158,7 +158,11 @@ async history storage and event-buffer cleanup keep WithoutCancel ancestry, then add final cancellation independently. History loops and delayed phone work keep BackgroundEventCtx ancestry. Every admitted task stays counted until its actual store, callback, send or handler returns, even if it ignores cancellation. -Ordinary Disconnect does not seal or cancel these tasks. +Ordinary Disconnect does not permanently seal the reusable client or apply +final cancellation to all these scopes. It retains its existing cancellation of +pending delayed phone requests whose owners are registered in the cancel map. +That cancellation does not prove actual return or join, and does not mean every +scheduled or admitted task has stopped. Synchronous ACK/name/storage branches retain their original inline execution and contexts, including canceled flags, NACKs and error policy. They are part of From 86789d67bab75b8ba6f129d5bef2d7ba4767dda7 Mon Sep 17 00:00:00 2001 From: Rajeh Taher Date: Thu, 22 Oct 2026 08:34:53 +0300 Subject: [PATCH 08/10] Own appstate missing-key and event batch lifetimes --- CLIENT_LIFETIME.md | 40 +++++- appstate.go | 14 +- appstate_lifecycle.go | 43 ++++++ appstate_lifecycle_test.go | 265 +++++++++++++++++++++++++++++++++++++ 4 files changed, 353 insertions(+), 9 deletions(-) create mode 100644 appstate_lifecycle.go create mode 100644 appstate_lifecycle_test.go diff --git a/CLIENT_LIFETIME.md b/CLIENT_LIFETIME.md index de8d68fab..621453da4 100644 --- a/CLIENT_LIFETIME.md +++ b/CLIENT_LIFETIME.md @@ -44,7 +44,9 @@ the deadline ends its waiter and does not manufacture completion. pruning, including the connect-success prune. See the exact boundaries below. - Client-owned message name/event/history/storage/receipt/key-share launches, ACK/retry handlers and delayed phone rerequests. Their exact contexts and loop - ownership are described below; appstate descendants remain separate gaps. + ownership are described below. The appstate children have their own boundary. +- Appstate missing-key request and the two contextless send/resync event batches, + including their actual synchronous callbacks and request/store return. - One teardown task, whose completion is recorded before launch. Context forwarding uses context.AfterFunc only to cancel contexts, with no user/protocol callbacks; those small cancellation callbacks are not a joined product-worker claim. @@ -198,6 +200,41 @@ canceling an entry is not its completion. Refused launch creates no entry. A synchronous DangerousInternals delayed call retains its Background behavior and is still caller-owned. No timers/retry counts/protocol algorithms changed. +## Appstate asynchronous children + +The missing-key request and both send/resync event batches register before +scheduling. The request keeps the original pre-launch WithoutCancel context +and PatchList pointer as typed helper parameters. Final cancellation is added +through the existing owned forwarding context. Refused admission does not enter +GetMissingKeyIDs, initialize/update/prune the request cache, advance request +times, log a key request or send. Admitted work stays counted through the actual +lookup/request return, even when a store or send ignores cancellation. The +request body, lock, deduplication, 24-hour cooldown, cap and ordering are unchanged. + +The two event-batch closures retain their original event-slice capture and range +order. A contextless batch owns every synchronous dispatch it makes until the +closure returns. Its private dispatcher keeps the existing handler-list locking, +callback status and panic recovery behavior, without reapplying the final seal +between already admitted members/handlers. This permits an admitted batch to +finish its original callbacks after sealing without dropping or reordering +members. New batches are refused after sealing. The general dispatchEvent +admission rules are unchanged; this private dispatcher must not escape to +independently spawned work. A user-spawned descendant is not joined by the batch. + +Ordinary Disconnect does not permanently seal the client or cancel these new +appstate children; its registered delayed-phone cancellation exception above +still applies. Batch reentry into bounded FinalClose cannot join its own callback +until it returns, so a waiter error cannot acknowledge completion. No new state +lock is held while the lifetime engine waits. The existing handler-list read lock +around synchronous callbacks is retained, including its existing reentry limits. + +Public FetchAppState/SendAppState and DangerousInternals synchronous operations +keep their signatures, errors and caller ownership. Only their named async +children gain this admission/join boundary. Appstate patch/decode/crypto/key/store +algorithms, retry recursion, protocol payloads and timers are unchanged. This is +not whole-client source closure, native acceptance, Signal ownership or a stop +receipt. Conservative established-context retention remains unchanged. + ## Still unbound The following existing client-owned producers retain their prior behavior and @@ -209,7 +246,6 @@ owning-library work before any successful stop result is possible. | Source at base 29a42459 | Unbound lifetime | | --- | --- | -| appstate.go:162,645,659 | Missing-key request and async event batches. | | pair.go:138-147 | Outer pair completion and PrePairCallback. Its unified-session child is now tracked; the parent and its other boundaries are not. | | handshake.go:98-103 | GetClientPayload callback admission is not separately sealed; an admitted connection can reach it. The enclosing connect is counted, which is not full callback-admission coverage. | diff --git a/appstate.go b/appstate.go index 6e870c331..f3819b40d 100644 --- a/appstate.go +++ b/appstate.go @@ -159,7 +159,7 @@ func (cli *Client) applyAppStatePatches( mutations, newState, err := cli.appStateProc.DecodePatches(ctx, patches, state, true) if err != nil { if errors.Is(err, appstate.ErrKeyNotFound) { - go cli.requestMissingAppStateKeys(context.WithoutCancel(ctx), patches) + cli.startMissingAppStateKeyRequest(context.WithoutCancel(ctx), patches) } else { cli.dispatchEvent(&events.AppStateSyncError{Name: name, FullSync: fullSync, Error: err}) } @@ -642,11 +642,11 @@ func (cli *Client) sendAppState(ctx context.Context, patch appstate.PatchInfo, a return fmt.Errorf("%w (also, applying patches in the response failed: %w)", mainErr, err) } else { zerolog.Ctx(ctx).Debug().Msg("Retrying app state send after applying conflicting patches") - go func() { + cli.startAppStateEventBatch(func(dispatch func(any) bool) { for _, evt := range eventsToDispatch { - cli.dispatchEvent(evt) + dispatch(evt) } - }() + }) return cli.sendAppState(ctx, patch, false) } } @@ -656,11 +656,11 @@ func (cli *Client) sendAppState(ctx context.Context, patch appstate.PatchInfo, a if err != nil { return fmt.Errorf("failed to fetch app state after sending update: %w", err) } - go func() { + cli.startAppStateEventBatch(func(dispatch func(any) bool) { for _, evt := range eventsToDispatch { - cli.dispatchEvent(evt) + dispatch(evt) } - }() + }) return nil } diff --git a/appstate_lifecycle.go b/appstate_lifecycle.go new file mode 100644 index 000000000..43a3e7c91 --- /dev/null +++ b/appstate_lifecycle.go @@ -0,0 +1,43 @@ +package whatsmeow + +import ( + "context" + "runtime/debug" + + "github.com/polymorfa/hypermeow/appstate" +) + +// Parameters copy the original direct go-call arguments before scheduling. +// Admission refusal must not run any key lookup, request-cache or send body. +func (cli *Client) startMissingAppStateKeyRequest(ctx context.Context, patches *appstate.PatchList) bool { + return cli.startConnectionTask(ctx, func(ctx context.Context) { cli.requestMissingAppStateKeys(ctx, patches) }) +} + +// The closure keeps the original event-slice capture and member order. A single +// admitted batch owns its synchronous dispatches through their actual return, +// including members reached after sealing. This dispatcher must not escape to +// independently spawned work. It grants no admission to user-spawned children. +func (cli *Client) startAppStateEventBatch(batch func(dispatch func(any) bool)) bool { + return cli.startNative(func() { batch(cli.dispatchAdmittedAppStateEvent) }) +} + +// Match dispatchEvent's synchronous handler-list, failure and panic semantics. +// The enclosing batch already owns admission; rechecking the seal per member or +// handler would truncate an admitted batch. The general dispatcher is unchanged. +func (cli *Client) dispatchAdmittedAppStateEvent(evt any) (handlerFailed bool) { + cli.eventHandlersLock.RLock() + defer func() { + cli.eventHandlersLock.RUnlock() + err := recover() + if err != nil { + cli.Log.Errorf("Event handler panicked while handling a %T: %v\n%s", evt, err, debug.Stack()) + } + }() + for _, handler := range cli.eventHandlers { + success := handler.fn(evt) + if !success { + return true + } + } + return false +} diff --git a/appstate_lifecycle_test.go b/appstate_lifecycle_test.go new file mode 100644 index 000000000..57e428293 --- /dev/null +++ b/appstate_lifecycle_test.go @@ -0,0 +1,265 @@ +package whatsmeow + +import ( + "context" + "errors" + "reflect" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/polymorfa/hypermeow/appstate" + "github.com/polymorfa/hypermeow/proto/waServerSync" + "github.com/polymorfa/hypermeow/store" +) + +type finalAppStateKeys struct { + store.NoopStore + calls atomic.Int32 + entered chan context.Context + release chan struct{} +} + +func (s *finalAppStateKeys) GetAppStateSyncKey(ctx context.Context, _ []byte) (*store.AppStateSyncKey, error) { + if s.calls.Add(1) > 1 { + s.entered <- ctx + <-s.release + } + return nil, nil +} +func finalAppStatePatches() *appstate.PatchList { + return &appstate.PatchList{Snapshot: &waServerSync.SyncdSnapshot{KeyID: &waServerSync.KeyId{ID: []byte("synthetic-key")}}} +} +func TestFinalAppStateMissingKeyActualStoreJoin(t *testing.T) { + keys := &finalAppStateKeys{entered: make(chan context.Context, 1), release: make(chan struct{})} + c := NewClient(&store.Device{AppStateKeys: keys}, nil) + var once sync.Once + release := func() { once.Do(func() { close(keys.release) }) } + t.Cleanup(func() { + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + parent, cancel := context.WithCancel(context.WithValue(context.Background(), finalConnectionContextKey{}, "appstate-value")) + defer cancel() + _, err := c.applyAppStatePatches(parent, appstate.WAPatchRegular, appstate.HashState{}, finalAppStatePatches(), false, nil) + if !errors.Is(err, appstate.ErrKeyNotFound) { + t.Fatalf("unexpected missing-key result: %v", err) + } + var ctx context.Context + select { + case ctx = <-keys.entered: + case <-time.After(2 * time.Second): + t.Fatal("missing-key scan store not reached") + } + cancel() + c.Disconnect() + if ctx.Err() != nil || ctx.Value(finalConnectionContextKey{}) != "appstate-value" { + t.Fatal("WithoutCancel ancestry or values changed") + } + finalTestBlocked(t, c) + finalTestWait(t, ctx.Done()) + if !errors.Is(ctx.Err(), context.Canceled) { + t.Fatal("final cancellation not forwarded") + } + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } +} + +func TestFinalAppStateMissingKeyRefusalDoesNotTouchBody(t *testing.T) { + for _, seeded := range []bool{false, true} { + t.Run(map[bool]string{false: "nil-cache", true: "seeded-cache"}[seeded], func(t *testing.T) { + keys := &finalAppStateKeys{entered: make(chan context.Context, 1), release: make(chan struct{})} + c := NewClient(&store.Device{AppStateKeys: keys}, nil) + var once sync.Once + release := func() { once.Do(func() { close(keys.release) }) } + t.Cleanup(func() { + release() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + stamp := time.Unix(123, 456) + if seeded { + c.appStateKeyRequests = map[string]time.Time{"old": stamp} + } + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + if c.startMissingAppStateKeyRequest(context.WithoutCancel(context.Background()), finalAppStatePatches()) { + t.Fatal("sealed key request admitted") + } + if keys.calls.Load() != 0 { + t.Fatal("refused task ran GetMissingKeyIDs/store") + } + if seeded { + if len(c.appStateKeyRequests) != 1 || c.appStateKeyRequests["old"] != stamp { + t.Fatal("refusal changed request cache/time") + } + } else if c.appStateKeyRequests != nil { + t.Fatal("refusal initialized request cache") + } + if finalContextWatcherCount(c) != 0 { + t.Fatal("refusal retained forwarding owner") + } + }) + } +} + +func TestFinalAppStateBatchMembersJoinAcrossSealAndReentry(t *testing.T) { + c := NewClient(&store.Device{}, nil) + first, second := make(chan struct{}), make(chan struct{}) + releaseFirst, releaseSecond := make(chan struct{}), make(chan struct{}) + reentering := make(chan struct{}) + selfResult := make(chan error, 1) + waiter, cancel := context.WithCancel(finalTestContext(t)) + defer cancel() + var once1, once2 sync.Once + unblock1 := func() { once1.Do(func() { close(releaseFirst) }) } + unblock2 := func() { once2.Do(func() { close(releaseSecond) }) } + t.Cleanup(func() { + cancel() + unblock1() + unblock2() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + var mu sync.Mutex + var order []string + var late atomic.Bool + record := func(v string) { mu.Lock(); order = append(order, v); mu.Unlock() } + c.AddEventHandler(func(evt any) { + member := evt.(string) + record("A" + member) + if member == "1" { + close(first) + <-releaseFirst + if c.startAppStateEventBatch(func(func(any) bool) { late.Store(true) }) { + late.Store(true) + } + close(reentering) + selfResult <- c.FinalClose(waiter) + } else if member == "2" { + close(second) + <-releaseSecond + } + }) + c.AddEventHandler(func(evt any) { record("B" + evt.(string)) }) + eventsToDispatch := []any{"1", "2"} + if !c.startAppStateEventBatch(func(dispatch func(any) bool) { + for _, evt := range eventsToDispatch { + dispatch(evt) + } + }) { + t.Fatal("batch refused") + } + finalTestWait(t, first) + c.Disconnect() + if c.nativeClosed() { + t.Fatal("ordinary Disconnect sealed client") + } + // The range header is already captured as in the original anonymous loop. + eventsToDispatch = []any{"changed"} + finalTestBlocked(t, c) + unblock1() + finalTestWait(t, reentering) + cancel() + select { + case err := <-selfResult: + if !errors.Is(err, context.Canceled) { + t.Fatalf("self waiter: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("reentrant FinalClose stuck") + } + finalTestWait(t, second) + finalTestBlocked(t, c) // The later member is still an actual held callback. + unblock2() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + mu.Lock() + got := append([]string(nil), order...) + mu.Unlock() + if !reflect.DeepEqual(got, []string{"A1", "B1", "A2", "B2"}) { + t.Fatalf("batch truncated or reordered: %v", got) + } + if late.Load() { + t.Fatal("reentrant post-seal batch admitted") + } +} +func TestFinalAppStateBatchRefusalAndOrdinaryReuse(t *testing.T) { + c := NewClient(&store.Device{}, nil) + t.Cleanup(func() { + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + var calls atomic.Int32 + done := make(chan struct{}, 2) + c.AddEventHandler(func(any) { calls.Add(1); done <- struct{}{} }) + launch := func() bool { return c.startAppStateEventBatch(func(dispatch func(any) bool) { dispatch("synthetic") }) } + for i := 0; i < 2; i++ { + c.Disconnect() + if !launch() { + t.Fatal("ordinary reusable client refused batch") + } + finalTestWait(t, done) + } + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + if launch() { + t.Fatal("post-seal batch admitted") + } + if calls.Load() != 2 { + t.Fatal("callback count changed on refusal") + } +} +func TestFinalAppStateBatchFailureAndPanicPreserveMemberOrder(t *testing.T) { + c := NewClient(&store.Device{}, nil) + t.Cleanup(func() { + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + var mu sync.Mutex + var order []string + record := func(v string) { mu.Lock(); order = append(order, v); mu.Unlock() } + c.AddEventHandlerWithSuccessStatus(func(evt any) bool { + member := evt.(string) + record("A" + member) + if member == "1" { + return false + } + if member == "2" { + panic("synthetic callback panic") + } + return true + }) + c.AddEventHandler(func(evt any) { record("B" + evt.(string)) }) + batchReturned := make(chan struct{}) + if !c.startAppStateEventBatch(func(dispatch func(any) bool) { + defer close(batchReturned) + for _, evt := range []any{"1", "2", "3"} { + dispatch(evt) + } + }) { + t.Fatal("batch refused") + } + finalTestWait(t, batchReturned) + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + mu.Lock() + got := append([]string(nil), order...) + mu.Unlock() + if !reflect.DeepEqual(got, []string{"A1", "A2", "A3", "B3"}) { + t.Fatalf("failure/panic semantics changed: %v", got) + } +} From 1362a94e20db6a10f78112f78d16f904a7335bbd Mon Sep 17 00:00:00 2001 From: Rajeh Taher Date: Thu, 22 Oct 2026 09:00:02 +0300 Subject: [PATCH 09/10] Own pairing task and client callback lifetimes --- CLIENT_LIFETIME.md | 69 ++++-- handshake.go | 8 +- pair.go | 10 +- pair_callback_lifecycle.go | 46 ++++ pair_callback_lifecycle_test.go | 388 ++++++++++++++++++++++++++++++++ 5 files changed, 500 insertions(+), 21 deletions(-) create mode 100644 pair_callback_lifecycle.go create mode 100644 pair_callback_lifecycle_test.go diff --git a/CLIENT_LIFETIME.md b/CLIENT_LIFETIME.md index 621453da4..d01f25655 100644 --- a/CLIENT_LIFETIME.md +++ b/CLIENT_LIFETIME.md @@ -47,6 +47,9 @@ the deadline ends its waiter and does not manufacture completion. ownership are described below. The appstate children have their own boundary. - Appstate missing-key request and the two contextless send/resync event batches, including their actual synchronous callbacks and request/store return. +- Outer pairing task through handlePair and its existing error/success + continuations; PrePair decision/callback and handshake payload selection/custom + callback through actual synchronous return. See the continuation boundary below. - One teardown task, whose completion is recorded before launch. Context forwarding uses context.AfterFunc only to cancel contexts, with no user/protocol callbacks; those small cancellation callbacks are not a joined product-worker claim. @@ -131,8 +134,8 @@ final cancellation and stay counted through the actual IQ/store/send return, including a store or transport that ignores cancellation. The synchronous DangerousInternals entry points keep their original signatures and Background semantics; they remain caller-owned. Both presence and pairing's unified child -launches are gated, but the outer pairing producer and PrePairCallback remain -unbound. No pairing body or protocol payload changed. +launches are gated. Outer pairing and PrePairCallback now have the separate +boundary below. No pairing algorithm or protocol payload changed. Expired-token pruning, including the connect-success child, registers before launch. Its existing TryLock guard coalesces work until the store actually @@ -235,19 +238,59 @@ algorithms, retry recursion, protocol payloads and timers are unchanged. This is not whole-client source closure, native acceptance, Signal ownership or a stop receipt. Conservative established-context retention remains unchanged. +## Outer pairing and client callbacks + +The outer handlePairSuccess closure registers with startConnectionTask before +scheduling. It keeps its original parsed local variables, props pointer and +caller-context ancestry/values, adding owned final cancellation. It remains +counted through handlePair and its original log/Disconnect/PairError or +unified-session/PairSuccess continuations. Refusal runs none of that async body. +The original synchronous parsing, time-offset update and possible props-parse +warning before launch are unchanged; this does not seal the entire caller API. + +PrePair callback decisions and handshake payload selection take a synchronous +native reservation before invocation. Nil PrePair still allows normally, false +still reaches the unchanged local-rejection branch, and the custom payload or +built-in Store.GetClientPayload fallback keeps its original value/bytes. A normal +nil custom payload remains nil; it is not replaced with a default. Refusal is +ErrClientClosed, so a sealed payload request cannot marshal an invented payload. +Both helpers release their count on actual return or panic, without swallowing +or translating the original panic policy. There is no implicit cancellation +argument for either contextless public callback signature. + +If final sealing is observed after the callback/selection returns, its result is +rejected with ErrClientClosed before the pairing persistence or handshake payload +marshalling continuation. A late true PrePair result is not accepted as a decision; +a late payload is not treated as success. The new closed branch is distinct from +the unchanged normal false rejection branch. A seal after that post-call check +can still race an already admitted continuation. The enclosing owned pair task +or connection remains counted until actual return; no rollback or arbitrary +revocation of already admitted store/protocol work is introduced. + +ConnectContext's existing closed checks and successful-context pin guard remain +intact and are not bypassed by the new callback error path. Already admitted +handshake continuations remain subject to those existing guards; no stronger +atomic no-effect or revocation contract is introduced. Public/DangerousInternals parents +beyond their named children remain separately caller-owned. This does not add +whole-request admission or authenticated ingress/custody. Callbacks can spawn +independent work whose completion is not established by their own return. + +No new state lock is held while waiting. Existing Connect holds its socket lock +while invoking the handshake callback, as before; FinalClose reentry from that +callback needs a bounded waiter and cannot join itself until it returns. The +waiter ending is never completion. Ordinary Disconnect remains reusable, with +its registered delayed-phone cancellation exception unchanged. Contextless +callbacks retain their original cancellation behavior and signatures. Client +callback fields gain no new concurrent-setter contract or copied/global owner. + ## Still unbound -The following existing client-owned producers retain their prior behavior and -are NOT included in the completion claim. Some use BackgroundEventCtx or -WithoutCancel and can continue launching child work after the core seal. This -is why the implementation cannot confirm full termination even when its tracked -count reaches zero. Their admission, cancellation and actual completion need -owning-library work before any successful stop result is possible. - -| Source at base 29a42459 | Unbound lifetime | -| --- | --- | -| pair.go:138-147 | Outer pair completion and PrePairCallback. Its unified-session child is now tracked; the parent and its other boundaries are not. | -| handshake.go:98-103 | GetClientPayload callback admission is not separately sealed; an admitted connection can reach it. The enclosing connect is counted, which is not full callback-admission coverage. | +The bounded source groups described above do not establish an exhaustive +producer inventory or complete client termination. Other caller/custom/callback +boundaries and the complete bounded successful-context inventory remain open. +No additional producer group or independent native acceptance was established +by these wrappers. FinalClose remains unconditionally incomplete after its +tracked join; missing ownership cannot be inferred from a zero tracked count. Caller-invoked public API methods, DangerousInternals, external HTTP transports, user store implementations and goroutines launched by user callbacks remain diff --git a/handshake.go b/handshake.go index 67bc7608b..108df7add 100644 --- a/handshake.go +++ b/handshake.go @@ -97,11 +97,9 @@ func (cli *Client) doHandshake(ctx context.Context, fs *socket.FrameSocket, ephe return fmt.Errorf("failed to mix noise private key in: %w", err) } - var clientPayload *waWa6.ClientPayload - if cli.GetClientPayload != nil { - clientPayload = cli.GetClientPayload() - } else { - clientPayload = cli.Store.GetClientPayload() + clientPayload, err := cli.clientPayloadForHandshake() + if err != nil { + return err } clientFinishPayloadBytes, err := proto.Marshal(clientPayload) diff --git a/pair.go b/pair.go index 5b8d1ccee..392a26eb3 100644 --- a/pair.go +++ b/pair.go @@ -135,7 +135,7 @@ func (cli *Client) handlePairSuccess(ctx context.Context, node *waBinary.Node) { cli.Log.Warnf("Failed to parse client pairing props: %v", err) } - go func() { + cli.startConnectionTask(ctx, func(ctx context.Context) { err := cli.handlePair(ctx, deviceIdentityBytes, id, businessName, platform, jid, lid) if err != nil { cli.Log.Errorf("Failed to pair device: %v", err) @@ -146,7 +146,7 @@ func (cli *Client) handlePairSuccess(ctx context.Context, node *waBinary.Node) { cli.startUnifiedSession() cli.dispatchEvent(&events.PairSuccess{ID: jid, LID: lid, BusinessName: businessName, Platform: platform, Props: &props}) } - }() + }) } func (cli *Client) handlePair(ctx context.Context, deviceIdentityBytes []byte, reqID, businessName, platform string, jid, lid types.JID) error { @@ -191,7 +191,11 @@ func (cli *Client) handlePair(ctx context.Context, deviceIdentityBytes []byte, r deviceIdentity.DeviceSignature = generateDeviceSignature(&deviceIdentity, cli.Store.IdentityKey)[:] - if cli.PrePairCallback != nil && !cli.PrePairCallback(jid, platform, businessName) { + allowed, err := cli.prePairCallbackDecision(jid, platform, businessName) + if err != nil { + return err + } + if !allowed { cli.sendPairError(ctx, reqID, 500, "internal-error") return ErrPairRejectedLocally } diff --git a/pair_callback_lifecycle.go b/pair_callback_lifecycle.go new file mode 100644 index 000000000..699a52104 --- /dev/null +++ b/pair_callback_lifecycle.go @@ -0,0 +1,46 @@ +package whatsmeow + +import ( + "github.com/polymorfa/hypermeow/proto/waWa6" + "github.com/polymorfa/hypermeow/types" +) + +// Admission owns this decision through actual callback return/panic. Nil keeps +// its normal allow behavior. A seal during invocation rejects the late decision +// before the existing pairing persistence/confirmation continuation is entered. +func (cli *Client) prePairCallbackDecision(jid types.JID, platform, businessName string) (bool, error) { + done, ok := cli.enterNative() + if !ok { + return false, ErrClientClosed + } + defer done() + allowed := true + if cli.PrePairCallback != nil { + allowed = cli.PrePairCallback(jid, platform, businessName) + } + if cli.nativeClosed() { + return false, ErrClientClosed + } + return allowed, nil +} + +// Keep the original custom callback or built-in fallback and exact payload. +// Refusal/late sealing is an error, never an invented payload to marshal. This +// owns the synchronous invocation, not user-spawned callback/store descendants. +func (cli *Client) clientPayloadForHandshake() (*waWa6.ClientPayload, error) { + done, ok := cli.enterNative() + if !ok { + return nil, ErrClientClosed + } + defer done() + var payload *waWa6.ClientPayload + if cli.GetClientPayload != nil { + payload = cli.GetClientPayload() + } else { + payload = cli.Store.GetClientPayload() + } + if cli.nativeClosed() { + return nil, ErrClientClosed + } + return payload, nil +} diff --git a/pair_callback_lifecycle_test.go b/pair_callback_lifecycle_test.go new file mode 100644 index 000000000..ee95bc718 --- /dev/null +++ b/pair_callback_lifecycle_test.go @@ -0,0 +1,388 @@ +package whatsmeow + +import ( + "bytes" + "context" + "errors" + "sync" + "testing" + "time" + + waBinary "github.com/polymorfa/hypermeow/binary" + "github.com/polymorfa/hypermeow/proto/waCompanionReg" + "github.com/polymorfa/hypermeow/proto/waWa6" + "github.com/polymorfa/hypermeow/store" + "github.com/polymorfa/hypermeow/types" + "github.com/polymorfa/hypermeow/types/events" + waLog "github.com/polymorfa/hypermeow/util/log" + "google.golang.org/protobuf/proto" +) + +type finalPairLog struct { + waLog.Logger + entered, release chan struct{} +} + +func (l *finalPairLog) Sub(string) waLog.Logger { return l } +func (l *finalPairLog) Errorf(msg string, args ...any) { + if msg == "Failed to pair device: %v" { + close(l.entered) + <-l.release + } +} +func finalPairFailureNode() *waBinary.Node { + return &waBinary.Node{Tag: "iq", Attrs: waBinary.Attrs{"id": "original-id", "t": "1"}, Content: []waBinary.Node{{Tag: "pair-success", Content: []waBinary.Node{ + {Tag: "device-identity", Content: []byte{0xff}}, // Rejected protobuf, before any key/crypto operation. + {Tag: "biz", Attrs: waBinary.Attrs{"name": "original-biz"}}, + {Tag: "platform", Attrs: waBinary.Attrs{"name": "original-platform"}}, + {Tag: "device", Attrs: waBinary.Attrs{"jid": types.NewJID("synthetic", types.DefaultUserServer)}}, + }}}} +} +func TestFinalPairOuterActualErrorContinuationJoin(t *testing.T) { + log := &finalPairLog{Logger: waLog.Noop, entered: make(chan struct{}), release: make(chan struct{})} + c := NewClient(&store.Device{}, log) + relay := &finalMessageRelay{fakeShadowRelay: &fakeShadowRelay{}, entered: make(chan context.Context, 1), release: make(chan struct{})} + c.shadowRelay = relay + var onceSend, onceLog sync.Once + releaseSend := func() { onceSend.Do(func() { close(relay.release) }) } + releaseLog := func() { onceLog.Do(func() { close(log.release) }) } + t.Cleanup(func() { + releaseSend() + releaseLog() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + parent := context.WithValue(context.Background(), finalConnectionContextKey{}, "pair-value") + c.handlePairSuccess(parent, finalPairFailureNode()) + var ctx context.Context + select { + case ctx = <-relay.entered: + case <-time.After(2 * time.Second): + t.Fatal("actual pair error send not reached") + } + c.Disconnect() + if ctx.Err() != nil || ctx.Value(finalConnectionContextKey{}) != "pair-value" { + t.Fatal("outer ancestry/value changed") + } + finalTestBlocked(t, c) + finalTestWait(t, ctx.Done()) + releaseSend() + finalTestWait(t, log.entered) + finalTestBlocked(t, c) // handlePair returned; its existing error continuation has not. + releaseLog() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } +} + +func TestFinalPairCallbackNormalValuesNilFalseAndPayload(t *testing.T) { + jid := types.NewJID("123", types.DefaultUserServer) + c := NewClient(&store.Device{ID: &jid}, nil) + t.Cleanup(func() { + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + allowed, err := c.prePairCallbackDecision(jid, "platform", "business") + if !allowed || err != nil { + t.Fatalf("nil callback: %v %v", allowed, err) + } + var calls int + c.PrePairCallback = func(got types.JID, platform, business string) bool { + calls++ + if got != jid || platform != "platform" || business != "business" { + t.Error("callback inputs changed") + } + return false + } + c.Disconnect() + allowed, err = c.prePairCallbackDecision(jid, "platform", "business") + if allowed || err != nil || calls != 1 { + t.Fatalf("normal false: %v %v calls%d", allowed, err, calls) + } + payload := &waWa6.ClientPayload{Username: proto.Uint64(321), Passive: proto.Bool(true)} + c.GetClientPayload = func() *waWa6.ClientPayload { return payload } + got, err := c.clientPayloadForHandshake() + if got != payload || err != nil { + t.Fatalf("custom payload changed: %p %v", got, err) + } + wantBytes, err := proto.Marshal(payload) + if err != nil { + t.Fatal(err) + } + gotBytes, err := proto.Marshal(got) + if err != nil || !bytes.Equal(wantBytes, gotBytes) { + t.Fatal("payload bytes changed") + } + c.GetClientPayload = nil + got, err = c.clientPayloadForHandshake() + if err != nil || !proto.Equal(got, c.Store.GetClientPayload()) { + t.Fatalf("built-in fallback changed: %v", err) + } + // A normal nil custom result remains nil; it is not replaced with fallback. + c.Store.ID = &types.EmptyJID + c.GetClientPayload = func() *waWa6.ClientPayload { return nil } + got, err = c.clientPayloadForHandshake() + if got != nil || err != nil { + t.Fatal("normal nil payload behavior changed") + } + if err = c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + c.PrePairCallback = func(types.JID, string, string) bool { t.Error("sealed PrePair called"); return true } + c.GetClientPayload = func() *waWa6.ClientPayload { t.Error("sealed payload called"); return payload } + if allowed, err = c.prePairCallbackDecision(jid, "platform", "business"); allowed || !errors.Is(err, ErrClientClosed) { + t.Fatalf("sealed PrePair: %v %v", allowed, err) + } + if got, err = c.clientPayloadForHandshake(); got != nil || !errors.Is(err, ErrClientClosed) { + t.Fatalf("sealed payload: %v %v", got, err) + } + c.GetClientPayload = nil // Empty ID would panic if refusal ran built-in fallback. + if got, err = c.clientPayloadForHandshake(); got != nil || !errors.Is(err, ErrClientClosed) { + t.Fatal("sealed request fell back") + } +} + +func TestFinalPairCallbackActualReturnReentryAndIsolation(t *testing.T) { + for _, kind := range []string{"prepair", "payload"} { + t.Run(kind, func(t *testing.T) { + c := NewClient(&store.Device{}, nil) + replacement := NewClient(&store.Device{}, nil) + entered, release, reentering := make(chan struct{}), make(chan struct{}), make(chan struct{}) + selfResult := make(chan error, 1) + result := make(chan error, 1) + waiter, cancel := context.WithCancel(finalTestContext(t)) + defer cancel() + var once sync.Once + unblock := func() { once.Do(func() { close(release) }) } + t.Cleanup(func() { + cancel() + unblock() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + if err := replacement.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + hook := func() { close(entered); <-release; close(reentering); selfResult <- c.FinalClose(waiter) } + payload := &waWa6.ClientPayload{Username: proto.Uint64(99)} + if kind == "prepair" { + c.PrePairCallback = func(types.JID, string, string) bool { hook(); return true } + go func() { + allowed, err := c.prePairCallbackDecision(types.EmptyJID, "p", "b") + if allowed && err == nil { + result <- errors.New("late true decision accepted") + } else { + result <- err + } + }() + } else { + c.GetClientPayload = func() *waWa6.ClientPayload { hook(); return payload } + go func() { + // Real Connect's existing caller holds this lock during doHandshake. + c.socketLock.Lock() + defer c.socketLock.Unlock() + got, err := c.clientPayloadForHandshake() + if got != nil { + result <- errors.New("late payload returned") + } else { + result <- err + } + }() + } + finalTestWait(t, entered) + if kind == "prepair" { + c.Disconnect() + } + // In the payload case ordinary Disconnect would wait on the existing caller's + // socket lock. It is tested separately outside that lock in the normal case. + finalTestBlocked(t, c) + replacement.PrePairCallback = func(types.JID, string, string) bool { return true } + replacement.GetClientPayload = func() *waWa6.ClientPayload { return payload } + if allowed, err := replacement.prePairCallbackDecision(types.EmptyJID, "other", "owner"); !allowed || err != nil { + t.Fatal("replacement PrePair affected") + } + if got, err := replacement.clientPayloadForHandshake(); got != payload || err != nil { + t.Fatal("replacement payload affected") + } + unblock() + finalTestWait(t, reentering) + cancel() + select { + case err := <-selfResult: + if !errors.Is(err, context.Canceled) { + t.Fatalf("self wait: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("callback reentry stuck") + } + select { + case err := <-result: + if !errors.Is(err, ErrClientClosed) { + t.Fatalf("late result: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("actual callback return not joined") + } + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + }) + } +} + +func TestFinalPairCallbackPanicsReleaseAdmission(t *testing.T) { + for _, kind := range []string{"prepair", "payload", "fallback"} { + t.Run(kind, func(t *testing.T) { + c := NewClient(&store.Device{}, nil) + t.Cleanup(func() { + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + c.PrePairCallback = func(types.JID, string, string) bool { panic("synthetic PrePair panic") } + c.GetClientPayload = func() *waWa6.ClientPayload { panic("synthetic payload panic") } + if kind == "fallback" { + c.GetClientPayload = nil + c.Store.ID = &types.EmptyJID + } + recovered := false + func() { + defer func() { recovered = recover() != nil }() + if kind == "prepair" { + _, _ = c.prePairCallbackDecision(types.EmptyJID, "p", "b") + } else { + _, _ = c.clientPayloadForHandshake() + } + }() + if !recovered { + t.Fatal("panic policy changed") + } + c.lifetime.mu.Lock() + active := c.lifetime.active + c.lifetime.mu.Unlock() + if active != 0 { + t.Fatal("panic stranded callback count") + } + }) + } +} + +type finalPairCaptureRelay struct { + *fakeShadowRelay + sent chan *waBinary.Node + contexts chan context.Context +} + +func (r *finalPairCaptureRelay) SendNode(ctx context.Context, data []byte) error { + raw, err := waBinary.Unpack(data) + if err != nil { + return err + } + node, err := waBinary.Unmarshal(raw) + if err != nil { + return err + } + r.sent <- node + r.contexts <- ctx + return nil +} +func TestFinalPairOuterPrelaunchCaptureAndCancelledContext(t *testing.T) { + c := NewClient(&store.Device{}, nil) + base, cancel := context.WithCancel(context.WithValue(context.Background(), finalConnectionContextKey{}, "captured-value")) + defer cancel() + parent := &finalMessageParent{Context: base, entered: make(chan struct{}), release: make(chan struct{})} + var once sync.Once + unblock := func() { once.Do(func() { close(parent.release) }) } + launched := make(chan struct{}) + t.Cleanup(func() { + unblock() + finalTestWait(t, launched) + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + relay := &finalPairCaptureRelay{fakeShadowRelay: &fakeShadowRelay{}, sent: make(chan *waBinary.Node, 1), contexts: make(chan context.Context, 1)} + c.shadowRelay = relay + event := make(chan *events.PairError, 1) + c.AddEventHandler(func(evt any) { + if v, ok := evt.(*events.PairError); ok { + event <- v + } + }) + node := finalPairFailureNode() + propsBytes, err := proto.Marshal(&waCompanionReg.ClientPairingProps{IsChatDbLidMigrated: proto.Bool(true)}) + if err != nil { + t.Fatal(err) + } + pairSuccess := node.GetChildByTag("pair-success") + children := pairSuccess.GetChildren() + children = append(children, waBinary.Node{Tag: "client-props", Content: propsBytes}) + node.Content = []waBinary.Node{{Tag: "pair-success", Content: children}} + go func() { c.handlePairSuccess(parent, node); close(launched) }() + finalTestWait(t, parent.entered) // Parsed locals/props exist; task is not scheduled. + node.Attrs["id"] = "replacement-id" + node.GetChildByTag("pair-success", "biz").Attrs["name"] = "replacement-biz" + node.GetChildByTag("pair-success", "platform").Attrs["name"] = "replacement-platform" + cancel() + c.Disconnect() + unblock() + finalTestWait(t, launched) + select { + case sent := <-relay.sent: + if sent.Attrs["id"] != "original-id" { + t.Fatal("reqID read moved after launch") + } + case <-time.After(2 * time.Second): + t.Fatal("actual error send absent") + } + select { + case ctx := <-relay.contexts: + if !errors.Is(ctx.Err(), context.Canceled) || ctx.Value(finalConnectionContextKey{}) != "captured-value" { + t.Fatal("caller cancellation/values changed") + } + case <-time.After(2 * time.Second): + t.Fatal("send context absent") + } + select { + case evt := <-event: + if evt.BusinessName != "original-biz" || evt.Platform != "original-platform" || !evt.Props.GetIsChatDbLidMigrated() { + t.Fatal("original local/props capture changed") + } + case <-time.After(2 * time.Second): + t.Fatal("PairError continuation absent") + } +} +func TestFinalPairOuterPostSealRefusesAsyncBody(t *testing.T) { + log := &finalPairLog{Logger: waLog.Noop, entered: make(chan struct{}), release: make(chan struct{})} + c := NewClient(&store.Device{}, log) + relay := &finalMessageRelay{fakeShadowRelay: &fakeShadowRelay{}, entered: make(chan context.Context, 1), release: make(chan struct{})} + c.shadowRelay = relay + var once sync.Once + unblock := func() { once.Do(func() { close(relay.release); close(log.release) }) } + t.Cleanup(func() { + unblock() + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Error(err) + } + }) + if err := c.FinalClose(finalTestContext(t)); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatal(err) + } + c.handlePairSuccess(context.Background(), finalPairFailureNode()) + select { + case <-relay.entered: + t.Fatal("refused outer task sent error") + default: + } + select { + case <-log.entered: + t.Fatal("refused outer task ran error continuation") + default: + } + if finalContextWatcherCount(c) != 0 { + t.Fatal("refused outer task retained context") + } +} From 8c575b680bb6850c634ea46243a65e167e3ff0ab Mon Sep 17 00:00:00 2001 From: Rajeh Taher Date: Thu, 22 Oct 2026 09:55:36 +0300 Subject: [PATCH 10/10] Join native cancellation forwarder actual return --- CLIENT_LIFETIME.md | 48 +++- client_forwarder.go | 71 ++++++ client_forwarder_test.go | 466 +++++++++++++++++++++++++++++++++++++++ client_lifecycle.go | 5 +- 4 files changed, 585 insertions(+), 5 deletions(-) create mode 100644 client_forwarder.go create mode 100644 client_forwarder_test.go diff --git a/CLIENT_LIFETIME.md b/CLIENT_LIFETIME.md index d01f25655..189e6c449 100644 --- a/CLIENT_LIFETIME.md +++ b/CLIENT_LIFETIME.md @@ -51,8 +51,9 @@ the deadline ends its waiter and does not manufacture completion. continuations; PrePair decision/callback and handshake payload selection/custom callback through actual synchronous return. See the continuation boundary below. - One teardown task, whose completion is recorded before launch. Context forwarding - uses context.AfterFunc only to cancel contexts, with no user/protocol callbacks; - those small cancellation callbacks are not a joined product-worker claim. + uses context.AfterFunc to cancel derived contexts. Exact completion entries + now join those existing forwarder invocations through actual cancel return; + stopping their registration alone is not completion. A permanent seal prevents new admissions to this subset. Work admitted before sealing may execute after sealing and remains counted until its function returns. @@ -69,8 +70,10 @@ background reconnect is requested, ConnectContext returns nil only when that reconnect task was actually admitted. Refused admission returns ErrClientClosed. The admitted task retains the context before scheduling and releases it on exit. Its internal attempts reuse the same forwarding scope instead of adding one -watcher per retry. Once no owner remains, cleanup unregisters the lifetime -AfterFunc and cancels the derived context. +watcher per retry. Once no owner remains, cleanup keeps its original lifetime +AfterFunc stop followed by derived cancel. Stopped registrations retire at once; +an already-started forwarder retains a separate completion entry until its actual +cancel invocation returns. A successful handshake transfers a context reference to the client lifetime before ConnectContext can return. These established contexts remain owned until @@ -87,6 +90,43 @@ subset, and releases the established-context references. No successful return or stop receipt follows from that cleanup. Tests of the context-transfer helper do not establish a live handshake/socket acceptance result. +## Forwarder actual-return ownership + +A nonzero-size entry on the exact client lifetime is registered before AfterFunc +can schedule its callback, including when an already admitted constructor reaches +a canceled lifetime. The callback keeps asynchronous AfterFunc behavior and calls +only the original derived cancel, with deferred entry completion. Its completion +does not read or depend on publication of the stop function. Stop-before-start +resolves that exact entry once; an already-running or completed callback makes +stop return false without pretending it joined. Entries retire on stopped-before- +start or actual callback completion, rather than retaining completed registrations. + +Entries are separate from active reservations and successful context pins. A +persistent registration counted as active would cycle with pins released only +after the active join. Final teardown retains its existing cancellation, +Disconnect and exact socket-close order, joins active work, detaches/releases +pins, then snapshots and drains remaining entry completions outside lifetime, +context, socket, store and global locks before closing stopped. Production +nativeContext constructors in ConnectContext, connect, startConnectionTask, +GetQRChannel and the synchronous history-loop adapter all have an active owner +before construction and release their context before completing that owner. +Therefore no production constructor can add a registration after that drain +frontier. Private helpers are not a new public admission contract. + +Ordinary last-reference cleanup does not acquire a new completion wait: it still +calls stop then cancel. Either call can have its existing blocking behavior, +including custom Context cleanup reached by cancel. Final teardown records an +already-started forwarder's actual return separately. A callback can finish before +its stop function is published, and bounded or canceled FinalClose waiters still +coalesce the same teardown. Reentry from a forwarder cannot join itself before +return and must end its bounded waiter without reporting completion. + +SOURCE-1 is locally implemented only, pending new independent review. It does +not bound successful-context retention, cancel established scopes earlier, join +user/custom/transport/Signal descendants or establish native/custody acceptance. +Callback or custom Context return does not prove its spawned descendants ended. +FinalClose remains always incomplete; no successful stop receipt follows. + ## QR channel closure and boundaries One reserved cleanup worker owns each output close and handler removal. Sends diff --git a/client_forwarder.go b/client_forwarder.go new file mode 100644 index 000000000..da0af6234 --- /dev/null +++ b/client_forwarder.go @@ -0,0 +1,71 @@ +package whatsmeow + +import ( + "context" + "sync" +) + +// A registration owns completion separately from active tasks and pinned scopes. +// Counting it as active would cycle with pins released only after the active join. +type nativeForwarder struct { + done chan struct{} + once sync.Once +} + +// registerForwarder is called while nativeContext's constructor has a reserved +// active owner. Publish the exact entry before AfterFunc can start, including +// when an admitted constructor reaches an already-canceled lifetime. +func (l *clientLifetime) registerForwarder(parent context.Context, cancel context.CancelFunc) func() bool { + return l.registerForwarderUsing(parent, cancel, context.AfterFunc) +} + +// The registrar argument permits a deterministic test of callback completion +// before its stop function is published. Production always uses context.AfterFunc. +func (l *clientLifetime) registerForwarderUsing(parent context.Context, cancel context.CancelFunc, after func(context.Context, func()) func() bool) func() bool { + entry := &nativeForwarder{done: make(chan struct{})} + l.mu.Lock() + if l.forwarders == nil { + l.forwarders = make(map[*nativeForwarder]struct{}) + } + l.forwarders[entry] = struct{}{} + l.mu.Unlock() + stop := after(parent, func() { + defer l.completeForwarder(entry) + cancel() + }) + // Callback completion does not depend on publication of this stop function. + // Go's stop contract excludes a callback start when stop returns true; false + // does not join a callback that has already started (or already completed). + return func() bool { + if stop() { + l.completeForwarder(entry) + return true + } + return false + } +} + +func (l *clientLifetime) completeForwarder(entry *nativeForwarder) { + entry.once.Do(func() { + l.mu.Lock() + delete(l.forwarders, entry) + close(entry.done) + l.mu.Unlock() + }) +} + +// Only finalDisconnect drains: after active owners join and pins release, every +// production nativeContext constructor has returned, so no new registration can +// appear. Ordinary last-reference cleanup retains stop(); cancel() without an +// added join. A custom Context can still block inside its own cancel cleanup. +func (l *clientLifetime) waitForwarders() { + l.mu.Lock() + pending := make([]<-chan struct{}, 0, len(l.forwarders)) + for entry := range l.forwarders { + pending = append(pending, entry.done) + } + l.mu.Unlock() + for _, done := range pending { + <-done + } +} diff --git a/client_forwarder_test.go b/client_forwarder_test.go new file mode 100644 index 000000000..31afb238e --- /dev/null +++ b/client_forwarder_test.go @@ -0,0 +1,466 @@ +package whatsmeow + +import ( + "context" + "errors" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/polymorfa/hypermeow/store" +) + +// WithCancel uses this parent's AfterFunc registration. The derived context's +// first real cancel reaches this blocking unregister after closing derived Done. +// A concurrent second cancel sees the already-canceled child and can return. +// This is a synthetic custom Context boundary, not a substituted CancelFunc. +type forwarderBlockingParent struct { + context.Context + never chan struct{} + entered, release chan struct{} + stopOnce, releaseOnce sync.Once +} + +func (p *forwarderBlockingParent) Done() <-chan struct{} { return p.never } +func (p *forwarderBlockingParent) AfterFunc(func()) func() bool { + return func() bool { + stopped := false + p.stopOnce.Do(func() { stopped = true; close(p.entered); <-p.release }) + return stopped + } +} +func (p *forwarderBlockingParent) unblock() { p.releaseOnce.Do(func() { close(p.release) }) } + +func forwarderWait(t *testing.T, ch <-chan struct{}) { + t.Helper() + select { + case <-ch: + case <-time.After(2 * time.Second): + t.Fatal("forwarder barrier not reached") + } +} +func forwarderClose(t *testing.T, c *Client) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + if err := c.FinalClose(ctx); !errors.Is(err, ErrClientShutdownIncomplete) { + t.Errorf("forwarder cleanup: %v", err) + } +} + +func TestForwarderActualCancelReturnJoinsFinalTeardown(t *testing.T) { + c := NewClient(&store.Device{}, nil) + p := &forwarderBlockingParent{Context: context.Background(), never: make(chan struct{}), entered: make(chan struct{}), release: make(chan struct{})} + var releaseContext, finishOwner func() + var releaseOnce, finishOnce sync.Once + release := func() { + if releaseContext != nil { + releaseOnce.Do(releaseContext) + } + } + finish := func() { + if finishOwner != nil { + finishOnce.Do(finishOwner) + } + } + // Install all releases before admission, scheduling or any fatal assertion. + t.Cleanup(func() { p.unblock(); release(); finish(); forwarderClose(t, c) }) + var ok bool + finishOwner, ok = c.enterNative() + if !ok { + t.Fatal("initial reservation refused") + } + var inherited context.Context + inherited, releaseContext = c.nativeContext(p) + waiter, cancelWaiter := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancelWaiter() + result := make(chan error, 1) + go func() { result <- c.FinalClose(waiter) }() + forwarderWait(t, p.entered) + forwarderWait(t, inherited.Done()) + if !errors.Is(inherited.Err(), context.Canceled) { + t.Fatal("actual derived cancellation missing") + } + released := make(chan struct{}) + go func() { release(); close(released) }() + forwarderWait(t, released) // Ordinary last-reference cleanup must not join the first cancel. + finish() + select { + case err := <-result: + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("final teardown completed before forwarding cancel returned: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("bounded final waiter hung") + } + p.unblock() + forwarderClose(t, c) +} + +// Done blocks construction before the registration, after startConnectionTask +// reserved its actual owner. Its values and ordinary cancellation stay intact. +type forwarderConstructionParent struct { + context.Context + entered, release chan struct{} + once, releaseOnce sync.Once +} + +func (p *forwarderConstructionParent) Done() <-chan struct{} { + p.once.Do(func() { close(p.entered); <-p.release }) + return p.Context.Done() +} +func (p *forwarderConstructionParent) unblock() { p.releaseOnce.Do(func() { close(p.release) }) } + +type forwarderValueKey struct{} + +func TestForwarderRegistrationAfterSealHasReservedTaskOwner(t *testing.T) { + c := NewClient(&store.Device{}, nil) + p := &forwarderConstructionParent{Context: context.WithValue(context.Background(), forwarderValueKey{}, "preserved"), entered: make(chan struct{}), release: make(chan struct{})} + taskRelease := make(chan struct{}) + var taskOnce sync.Once + unblockTask := func() { taskOnce.Do(func() { close(taskRelease) }) } + t.Cleanup(func() { p.unblock(); unblockTask(); forwarderClose(t, c) }) + inherited := make(chan context.Context, 1) + launched := make(chan bool, 1) + go func() { + launched <- c.startConnectionTask(p, func(ctx context.Context) { inherited <- ctx; <-taskRelease }) + }() + forwarderWait(t, p.entered) + waiter, cancel := context.WithCancel(finalTestContext(t)) + defer cancel() + result := make(chan error, 1) + go func() { result <- c.FinalClose(waiter) }() + forwarderWait(t, c.nativeDone()) // Establish cancellation-before-registration, not derived Err ordering. + if !c.nativeClosed() { + t.Fatal("constructor barrier did not follow final seal") + } + p.unblock() + var ctx context.Context + select { + case ctx = <-inherited: + case <-time.After(2 * time.Second): + t.Fatal("reserved task did not run") + } + if !<-launched { + t.Fatal("already reserved task was refused") + } + forwarderWait(t, ctx.Done()) + if !errors.Is(ctx.Err(), context.Canceled) || ctx.Value(forwarderValueKey{}) != "preserved" { + t.Fatal("late registration lost cancellation or values") + } + cancel() + if err := <-result; !errors.Is(err, context.Canceled) { + t.Fatalf("blocked task waiter: %v", err) + } + c.lifetime.mu.Lock() + active := c.lifetime.active + c.lifetime.mu.Unlock() + if active != 1 { + t.Fatalf("reserved task not retained: %d", active) + } + unblockTask() + forwarderClose(t, c) + if forwarderCount(c) != 0 { + t.Fatal("late registration retained completed entry") + } +} + +func forwarderCount(c *Client) int { + c.lifetime.mu.Lock() + defer c.lifetime.mu.Unlock() + return len(c.lifetime.forwarders) +} +func forwarderEntry(c *Client) *nativeForwarder { + c.lifetime.mu.Lock() + defer c.lifetime.mu.Unlock() + for entry := range c.lifetime.forwarders { + return entry + } + return nil +} + +func TestForwarderStopAndPublicationStates(t *testing.T) { + t.Run("stop-before-start-concurrent", func(t *testing.T) { + c := NewClient(&store.Device{}, nil) + var finish func() + var finishOnce sync.Once + t.Cleanup(func() { + if finish != nil { + finishOnce.Do(finish) + } + forwarderClose(t, c) + }) + var ok bool + finish, ok = c.enterNative() + if !ok { + t.Fatal("reservation refused") + } + var calls atomic.Int32 + stop := c.lifetime.registerForwarder(c.lifetime.ctx, func() { calls.Add(1) }) + entry := forwarderEntry(c) + if entry == nil { + t.Fatal("entry not registered before scheduling") + } + stopped := make(chan bool, 4) + for i := 0; i < 4; i++ { + go func() { stopped <- stop() }() + } + winners := 0 + for i := 0; i < 4; i++ { + if <-stopped { + winners++ + } + } + forwarderWait(t, entry.done) + if winners != 1 || calls.Load() != 0 || forwarderCount(c) != 0 { + t.Fatalf("stop ownership: winners%d calls%d entries%d", winners, calls.Load(), forwarderCount(c)) + } + finishOnce.Do(finish) + forwarderClose(t, c) + if calls.Load() != 0 || stop() { + t.Fatal("stopped registration ran or resurrected") + } + }) + t.Run("already-completed", func(t *testing.T) { + c := NewClient(&store.Device{}, nil) + var finish func() + var finishOnce sync.Once + t.Cleanup(func() { + if finish != nil { + finishOnce.Do(finish) + } + forwarderClose(t, c) + }) + var ok bool + finish, ok = c.enterNative() + if !ok { + t.Fatal("reservation refused") + } + parent, cancel := context.WithCancel(context.Background()) + defer cancel() + var calls atomic.Int32 + stop := c.lifetime.registerForwarder(parent, func() { calls.Add(1) }) + entry := forwarderEntry(c) + if entry == nil { + t.Fatal("entry missing") + } + cancel() + forwarderWait(t, entry.done) + if stop() || calls.Load() != 1 || forwarderCount(c) != 0 { + t.Fatal("completed callback stop/retirement changed") + } + finishOnce.Do(finish) + }) + t.Run("callback-before-stop-publication", func(t *testing.T) { + c := NewClient(&store.Device{}, nil) + var finish func() + var finishOnce sync.Once + t.Cleanup(func() { + if finish != nil { + finishOnce.Do(finish) + } + forwarderClose(t, c) + }) + var ok bool + finish, ok = c.enterNative() + if !ok { + t.Fatal("reservation refused") + } + parent, cancel := context.WithCancel(context.Background()) + cancel() + var calls atomic.Int32 + published := false + stop := c.lifetime.registerForwarderUsing(parent, func() { calls.Add(1) }, func(ctx context.Context, fn func()) func() bool { + // Hold only registrar result publication. Real stdlib AfterFunc still owns + // asynchronous scheduling; the callback body and retirement are production. + entry := forwarderEntry(c) + if entry == nil { + t.Fatal("registration not published before AfterFunc") + } + underlying := context.AfterFunc(ctx, fn) + forwarderWait(t, entry.done) + if forwarderCount(c) != 0 || calls.Load() != 1 { + t.Fatal("callback depended on stop publication") + } + published = true + return underlying + }) + if !published || stop() || forwarderCount(c) != 0 { + t.Fatal("late stop publication resurrected entry") + } + finishOnce.Do(finish) + }) +} + +func TestForwarderPinnedReuseAndForeignLifetimeIsolation(t *testing.T) { + c, foreign := NewClient(&store.Device{}, nil), NewClient(&store.Device{}, nil) + var releaseAttempt, finishAttempt, releaseForeign, finishForeign func() + var attemptOnce, ownerOnce, foreignOnce, foreignOwnerOnce sync.Once + releaseA := func() { + if releaseAttempt != nil { + attemptOnce.Do(releaseAttempt) + } + } + finishA := func() { + if finishAttempt != nil { + ownerOnce.Do(finishAttempt) + } + } + releaseB := func() { + if releaseForeign != nil { + foreignOnce.Do(releaseForeign) + } + } + finishB := func() { + if finishForeign != nil { + foreignOwnerOnce.Do(finishForeign) + } + } + t.Cleanup(func() { + releaseB() + finishB() + releaseA() + finishA() + forwarderClose(t, foreign) + forwarderClose(t, c) + }) + var ok bool + finishAttempt, ok = c.enterNative() + if !ok { + t.Fatal("attempt reservation refused") + } + parent := context.WithValue(context.Background(), forwarderValueKey{}, "retained") + var ctx context.Context + ctx, releaseAttempt = c.nativeContext(parent) + if !c.pinNativeContext(ctx) { + t.Fatal("pin refused") + } + releaseA() + finishA() + c.Disconnect() + if ctx.Err() != nil || forwarderCount(c) != 1 { + t.Fatal("ordinary release/disconnect canceled successful scope") + } + // Model two simultaneous inherited attempt owners, rather than another new + // successful plain-caller scope. Both must retain/reuse the exact pinned scope. + done, ok := c.enterNative() + if !ok { + t.Fatal("reconnect reservation refused") + } + var doneOnce sync.Once + t.Cleanup(func() { doneOnce.Do(done) }) + a, release1 := c.nativeContext(ctx) + b, release2 := c.nativeContext(ctx) + var r1, r2 sync.Once + t.Cleanup(func() { r1.Do(release1); r2.Do(release2) }) + if a != ctx || b != ctx || forwarderCount(c) != 1 { + t.Fatal("inherited attempts allocated another registration") + } + returned := make(chan struct{}, 2) + go func() { r1.Do(release1); returned <- struct{}{} }() + go func() { r2.Do(release2); returned <- struct{}{} }() + <-returned + <-returned + doneOnce.Do(done) + if ctx.Err() != nil { + t.Fatal("inherited attempt release canceled pin") + } + finishForeign, ok = foreign.enterNative() + if !ok { + t.Fatal("foreign reservation refused") + } + var other context.Context + other, releaseForeign = foreign.nativeContext(ctx) + if other == ctx || !foreign.pinNativeContext(other) { + t.Fatal("foreign client reused exact owner or pin refused") + } + releaseB() + finishB() + forwarderClose(t, foreign) + forwarderWait(t, other.Done()) + if !errors.Is(other.Err(), context.Canceled) || ctx.Err() != nil || other.Value(forwarderValueKey{}) != "retained" { + t.Fatal("foreign cancellation escaped its derived scope") + } + forwarderClose(t, c) + forwarderWait(t, ctx.Done()) + if !errors.Is(ctx.Err(), context.Canceled) || forwarderCount(c) != 0 || forwarderCount(foreign) != 0 { + t.Fatal("pin/registration cleanup did not join") + } + c.lifetime.mu.Lock() + active, pins := c.lifetime.active, len(c.lifetime.contexts) + c.lifetime.mu.Unlock() + if active != 0 || pins != 0 { + t.Fatalf("pin/active cycle or retention: %d/%d", active, pins) + } +} + +func TestForwarderReentrantAndConcurrentFinalWaiters(t *testing.T) { + c := NewClient(&store.Device{}, nil) + var finish func() + var finishOnce sync.Once + t.Cleanup(func() { + if finish != nil { + finishOnce.Do(finish) + } + forwarderClose(t, c) + }) + var ok bool + finish, ok = c.enterNative() + if !ok { + t.Fatal("reservation refused") + } + entered := make(chan struct{}) + reentry := make(chan error, 1) + c.lifetime.registerForwarder(c.lifetime.ctx, func() { + close(entered) + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + reentry <- c.FinalClose(ctx) + }) + finishOnce.Do(finish) + waiter, cancel := context.WithCancel(finalTestContext(t)) + defer cancel() + canceled := make(chan error, 1) + go func() { canceled <- c.FinalClose(waiter) }() + forwarderWait(t, entered) + c.lifetime.mu.Lock() + originalStop := c.lifetime.stopped + c.lifetime.mu.Unlock() + cancel() + if err := <-canceled; !errors.Is(err, context.Canceled) { + t.Fatalf("canceled waiter: %v", err) + } + results := make(chan error, 2) + for i := 0; i < 2; i++ { + ctx := finalTestContext(t) + go func() { results <- c.FinalClose(ctx) }() + } + if err := <-reentry; !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("self-forwarder wait confirmed completion: %v", err) + } + for i := 0; i < 2; i++ { + if err := <-results; !errors.Is(err, ErrClientShutdownIncomplete) { + t.Fatalf("coalesced result: %v", err) + } + } + forwarderClose(t, c) + c.lifetime.mu.Lock() + same := c.lifetime.stopped == originalStop + c.lifetime.mu.Unlock() + if !same || forwarderCount(c) != 0 { + t.Fatal("waiter cancellation retried teardown or retained callback") + } + replacement := NewClient(&store.Device{}, nil) + t.Cleanup(func() { forwarderClose(t, replacement) }) + ran := make(chan struct{}) + if !replacement.startConnectionTask(context.Background(), func(ctx context.Context) { + if ctx.Err() != nil { + t.Error("replacement canceled") + } + close(ran) + }) { + t.Fatal("old final close sealed replacement") + } + forwarderWait(t, ran) +} diff --git a/client_lifecycle.go b/client_lifecycle.go index 970e887e0..64886987b 100644 --- a/client_lifecycle.go +++ b/client_lifecycle.go @@ -29,6 +29,8 @@ type clientLifetime struct { stopped chan struct{} sockets map[*socket.FrameSocket]int contexts map[*nativeClientContext]struct{} + + forwarders map[*nativeForwarder]struct{} } func (l *clientLifetime) initLocked() { @@ -181,7 +183,7 @@ func (cli *Client) nativeContext(parent context.Context) (context.Context, conte lifetime := l.ctx l.mu.Unlock() ctx, cancel := context.WithCancel(parent) - stop := context.AfterFunc(lifetime, cancel) + stop := l.registerForwarder(lifetime, cancel) owned := &nativeClientContext{Context: ctx, lifetime: l, refs: 1, cleanup: func() { stop(); cancel() }} return owned, owned.release } @@ -296,5 +298,6 @@ func (cli *Client) finalDisconnect() { for owned := range contexts { owned.release() } + l.waitForwarders() close(l.stopped) }