fix(network): fix goroutine leak in waitTimeout

waitTimeout spawned a goroutine blocked on wg.Wait() and used an
unbuffered channel. When the timeout fired the function returned but
the goroutine stayed alive forever, one leak per timeout event.

Fix: use a buffered channel (capacity 1) so the goroutine can always
send and exit once wg reaches zero, even after the caller has returned.
Accept a context.Context so cancellation is also propagated correctly.

Fixes #2124

Signed-off-by: AkramBitar <akram@il.ibm.com>

Author: AkramBitar

token/services/network/fabric/network.go

@@ -418,7 +418,7 @@ func (n *Network) LookupTransferMetadataKey(namespace string, key string, timeou
 			logger.Debugf("failed to remove lookup listener [%s]: %v", transferMetadataKey, err)
 		}
 	}()
-	if err := waitTimeout(wg, timeout); err != nil {
+	if err := waitTimeout(context.Background(), wg, timeout); err != nil {
 		return nil, err
 	}
 	logger.Debugf("lookup transfer metadata key [%s] from [%s] in namespace [%s], done, result [%s][%s]", key, transferMetadataKey, namespace, l.value, l.err)
@@ -579,17 +579,39 @@ func (l *lookupListener) OnError(ctx context.Context, key string, err error) {
 	}
 }
 
-func waitTimeout(wg *sync.WaitGroup, timeout time.Duration) error {
-	c := make(chan struct{})
+// waitTimeout waits for wg to reach zero or for the timeout to expire,
+// whichever comes first.
+//
+// The root cause of the previous goroutine leak (issue #2124): the old
+// implementation used an unbuffered channel. When the timeout fired,
+// waitTimeout returned and nobody was left to read from the channel, so
+// the goroutine stayed blocked on wg.Wait()+close(c) forever.
+//
+// Fix: the channel has capacity 1. When wg.Wait() eventually returns the
+// goroutine sends one value and exits immediately, regardless of whether
+// waitTimeout has already returned. The send never blocks because the
+// buffer absorbs it.
+//
+// Note: sync.WaitGroup has no context-aware Wait, so the goroutine cannot
+// be cancelled mid-flight. It will remain alive until wg reaches zero
+// (i.e. until the owner calls wg.Done()), but it will always exit after that.
+func waitTimeout(ctx context.Context, wg *sync.WaitGroup, timeout time.Duration) error {
+	ctx, cancel := context.WithTimeout(ctx, timeout)
+	defer cancel()
+
+	// Buffered (capacity 1): the goroutine can always send even if this
+	// function has already returned via the timeout/cancel path.
+	done := make(chan struct{}, 1)
 	go func() {
-		defer close(c)
 		wg.Wait()
+		done <- struct{}{}
 	}()
+
 	select {
-	case <-c:
+	case <-done:
 		return nil
-	case <-time.After(timeout):
-		return errors.Errorf("context done")
+	case <-ctx.Done():
+		return ctx.Err()
 	}
 }
 

token/services/network/fabric/wait_timeout_test.go

@@ -0,0 +1,127 @@
+/*
+Copyright IBM Corp. All Rights Reserved.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package fabric
+
+import (
+	"context"
+	"runtime"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestWaitTimeout_CompletesBeforeTimeout verifies the happy path: wg reaches zero
+// before the deadline and waitTimeout returns nil with no goroutine left behind.
+func TestWaitTimeout_CompletesBeforeTimeout(t *testing.T) {
+	wg := &sync.WaitGroup{}
+	wg.Add(1)
+
+	go func() {
+		time.Sleep(10 * time.Millisecond)
+		wg.Done()
+	}()
+
+	err := waitTimeout(context.Background(), wg, 5*time.Second)
+	require.NoError(t, err)
+}
+
+// TestWaitTimeout_TimeoutFires verifies that waitTimeout returns an error when
+// the WaitGroup does not reach zero within the timeout.
+func TestWaitTimeout_TimeoutFires(t *testing.T) {
+	wg := &sync.WaitGroup{}
+	wg.Add(1) // never Done'd — simulates a stuck operation
+
+	err := waitTimeout(context.Background(), wg, 50*time.Millisecond)
+	require.Error(t, err)
+
+	// Unblock the goroutine left behind so the test and runtime clean up.
+	wg.Done()
+}
+
+// TestWaitTimeout_ContextCancelled verifies that a cancelled context causes
+// waitTimeout to return immediately with context.Canceled, even when the
+// timeout has not expired.
+func TestWaitTimeout_ContextCancelled(t *testing.T) {
+	wg := &sync.WaitGroup{}
+	wg.Add(1) // never Done'd
+
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel() // cancel immediately
+
+	err := waitTimeout(ctx, wg, 10*time.Second) // long timeout — must not block
+	require.ErrorIs(t, err, context.Canceled)
+
+	wg.Done() // clean up the stuck goroutine
+}
+
+// TestWaitTimeout_NoGoroutineLeak is the regression test for issue #2124.
+//
+// Before the fix, waitTimeout used an unbuffered channel and a bare goroutine
+// that parked on wg.Wait() with no way to be signalled. When the timeout fired
+// the function returned but the goroutine remained alive forever, one per
+// timeout event.
+//
+// After the fix the channel is buffered (capacity 1), so the goroutine can
+// always send and exit once wg reaches zero, even if waitTimeout has already
+// returned. This test confirms that the goroutine count returns to its
+// pre-call baseline within a short grace period after the wg is released.
+func TestWaitTimeout_NoGoroutineLeak(t *testing.T) {
+	// Stabilise the baseline goroutine count before the test.
+	runtime.GC()
+	time.Sleep(10 * time.Millisecond)
+	before := runtime.NumGoroutine()
+
+	wg := &sync.WaitGroup{}
+	wg.Add(1)
+
+	// Fire the timeout — this is the path that previously leaked.
+	err := waitTimeout(context.Background(), wg, 20*time.Millisecond)
+	require.Error(t, err, "expected timeout error")
+
+	// Release the WaitGroup so the background goroutine can exit.
+	wg.Done()
+
+	// Give the runtime a moment to schedule and collect the goroutine.
+	assert.Eventually(t, func() bool {
+		runtime.GC()
+		return runtime.NumGoroutine() <= before+1 // +1 for test framework variance
+	}, 2*time.Second, 50*time.Millisecond,
+		"goroutine leaked: count did not return to baseline after wg.Done()")
+}
+
+// TestWaitTimeout_MultipleTimeouts verifies that repeated timeout events do not
+// accumulate goroutines — each goroutine exits once its wg is released.
+func TestWaitTimeout_MultipleTimeouts(t *testing.T) {
+	const n = 10
+
+	runtime.GC()
+	time.Sleep(10 * time.Millisecond)
+	before := runtime.NumGoroutine()
+
+	wgs := make([]*sync.WaitGroup, n)
+	for i := range n {
+		wg := &sync.WaitGroup{}
+		wg.Add(1)
+		wgs[i] = wg
+		err := waitTimeout(context.Background(), wg, 20*time.Millisecond)
+		require.Error(t, err)
+	}
+
+	// Release all WaitGroups so all background goroutines can exit.
+	for _, wg := range wgs {
+		wg.Done()
+	}
+
+	assert.Eventually(t, func() bool {
+		runtime.GC()
+		return runtime.NumGoroutine() <= before+1
+	}, 2*time.Second, 50*time.Millisecond,
+		"goroutines leaked after %d timeout events", n)
+}