fix: Avoid locking on app lifecycle change

Author: alexjba

pkg/backend/geth_backend.go

@@ -2089,20 +2089,26 @@ func (b *StatusBackend) getVerifiedWalletAccount(address, password string) (*gen
 // AppStateChange handles app state changes (background/foreground).
 // state values: see https://facebook.github.io/react-native/docs/appstate.html
 func (b *StatusBackend) AppStateChange(state AppState) {
-	b.mu.Lock()
-	defer b.mu.Unlock()
-
 	if !state.IsValid() {
 		b.logger.Warn("invalid app state, not reporting app state change", zap.Any("state", state))
 		return
 	}
 
-	b.appState = state
+	b.mu.Lock()
+	defer b.mu.Unlock()
 
+	b.appState = state
 	if b.statusNode == nil {
 		b.logger.Warn("statusNode nil, applying app state change without running node")
 	}
 
+	if state == AppStateForeground && b.lifecycleState == AppLifecycleRunning {
+		return
+	}
+	if state != AppStateForeground && b.lifecycleState == AppLifecyclePausedBackground {
+		return
+	}
+
 	if state == AppStateForeground {
 		if err := b.resumeLocked(); err != nil {
 			b.logger.Warn("failed to resume backend on app foreground", zap.Error(err))

pkg/backend/pause_play.go

@@ -3,6 +3,7 @@ package backend
 import (
 	"fmt"
 
+	"github.com/status-im/status-go/pkg/backend/node"
 	"github.com/status-im/status-go/protocol"
 )
 
@@ -18,59 +19,66 @@ const (
 
 func (s AppLifecycleState) String() string { return string(s) }
 
+// LifecycleState returns the current backend lifecycle state.
 func (b *StatusBackend) LifecycleState() AppLifecycleState {
 	b.mu.Lock()
 	defer b.mu.Unlock()
 	return b.lifecycleState
 }
 
+// pauseLocked must be called with b.mu held.
 func (b *StatusBackend) pauseLocked() error {
 	if b.lifecycleState == AppLifecycleStopped {
 		return nil
 	}
 	if b.lifecycleState == AppLifecyclePausedBackground {
 		return nil
 	}
-	if b.statusNode == nil || !b.statusNode.IsRunning() {
+	sn := b.statusNode
+
+	if sn == nil || !sn.IsRunning() {
 		b.lifecycleState = AppLifecycleStopped
 		return nil
 	}
-	if messenger := b.currentMessengerLocked(); messenger != nil {
+	if messenger := b.currentMessenger(sn); messenger != nil {
 		messenger.ToBackground()
 	}
-	if err := b.statusNode.PauseBackground(); err != nil {
+	if err := sn.PauseBackground(); err != nil {
 		return fmt.Errorf("pause background: %w", err)
 	}
 	b.lifecycleState = AppLifecyclePausedBackground
 	return nil
 }
 
+// resumeLocked must be called with b.mu held.
 func (b *StatusBackend) resumeLocked() error {
 	if b.lifecycleState == AppLifecycleStopped {
 		return nil
 	}
 	if b.lifecycleState == AppLifecycleRunning {
 		return nil
 	}
-	if b.statusNode == nil || !b.statusNode.IsRunning() {
+	sn := b.statusNode
+
+	if sn == nil || !sn.IsRunning() {
 		b.lifecycleState = AppLifecycleStopped
 		return nil
 	}
-	if messenger := b.currentMessengerLocked(); messenger != nil {
+	if messenger := b.currentMessenger(sn); messenger != nil {
 		messenger.ToForeground()
 	}
 
 	b.lifecycleState = AppLifecycleResumingForeground
-	if err := b.statusNode.ResumeForeground(); err != nil {
+	if err := sn.ResumeForeground(); err != nil {
 		return fmt.Errorf("resume foreground: %w", err)
 	}
 	b.lifecycleState = AppLifecycleRunning
 	return nil
 }
 
-func (b *StatusBackend) currentMessengerLocked() *protocol.Messenger {
-	if b.statusNode == nil || b.statusNode.WakuV2ExtService() == nil {
+func (b *StatusBackend) currentMessenger(sn *node.StatusNode) *protocol.Messenger {
+	if sn == nil || sn.WakuV2ExtService() == nil {
 		return nil
 	}
-	return b.statusNode.WakuV2ExtService().Messenger()
+	return sn.WakuV2ExtService().Messenger()
 }

pkg/messaging/layers/reliability/datasync/transport.go

@@ -33,7 +33,7 @@ func SetPausedBackground(paused bool) {
 func currentOffsetToSecond() uint64 {
 	datasyncTickerMutex.RLock()
 	defer datasyncTickerMutex.RUnlock()
-	return uint64(time.Second / DatasyncTicker)
+	return uint64(math.Ceil(float64(time.Second) / float64(DatasyncTicker)))
 }
 
 type NodeTransport struct {

server/server.go

@@ -10,6 +10,7 @@ import (
 	"net/url"
 	"strconv"
 	"sync"
+	"sync/atomic"
 	"syscall"
 	"time"
 
@@ -29,7 +30,7 @@ type Config struct {
 }
 
 type Server struct {
-	lifecycleMu sync.Mutex
+	mu sync.Mutex
 
 	listener net.Listener
 	server   *http.Server
@@ -41,8 +42,8 @@ type Server struct {
 	// address is the host and port the server is listening on
 	address *net.TCPAddr
 
-	// isRunning is true if the server was started and is running
-	isRunning bool
+	// running is true if the server was started and Serve is active; read via IsRunning().
+	running atomic.Bool
 
 	// cachedPort stores the port from the first successful bind when AddrPort used port 0 (ephemeral).
 	// Reused on pause/resume so URLs remain valid across ToBackground/ToForeground cycles.
@@ -159,13 +160,13 @@ func (s *Server) serve(currentServer *http.Server, currentListener net.Listener)
 	defer common.LogOnPanic()
 
 	defer func() {
-		s.lifecycleMu.Lock()
-		defer s.lifecycleMu.Unlock()
+		s.mu.Lock()
+		defer s.mu.Unlock()
 
 		// If a newer Start() already replaced server/listener, do not clobber
 		// the latest running instance state.
 		if s.server == currentServer && s.listener == currentListener {
-			s.isRunning = false
+			s.running.Store(false)
 			s.address = nil
 		}
 	}()
@@ -199,10 +200,10 @@ func (s *Server) applyHandlers() {
 }
 
 func (s *Server) Start() error {
-	s.lifecycleMu.Lock()
-	defer s.lifecycleMu.Unlock()
+	s.mu.Lock()
+	defer s.mu.Unlock()
 
-	if s.isRunning {
+	if s.running.Load() {
 		return nil
 	}
 
@@ -217,31 +218,31 @@ func (s *Server) Start() error {
 
 	// Mark running synchronously to avoid pause/play races where ToBackground
 	// can run before serve() goroutine has a chance to set the state.
-	s.isRunning = true
+	s.running.Store(true)
 	go s.serve(s.server, s.listener)
 	return nil
 }
 
 func (s *Server) Stop() error {
-	s.lifecycleMu.Lock()
+	s.mu.Lock()
 	s.StopTimeout()
-	if !s.isRunning || s.server == nil {
-		s.lifecycleMu.Unlock()
+	if !s.running.Load() || s.server == nil {
+		s.mu.Unlock()
 		return nil
 	}
 
 	// Capture the current instance and release the lock before Shutdown.
 	// Shutdown waits for Serve() to return, and Serve() may update state in
 	// its defer path under the same mutex.
 	currentServer := s.server
-	s.isRunning = false
-	s.lifecycleMu.Unlock()
+	s.running.Store(false)
+	s.mu.Unlock()
 
 	return currentServer.Shutdown(context.Background())
 }
 
 func (s *Server) IsRunning() bool {
-	return s.isRunning
+	return s.running.Load()
 }
 
 func (s *Server) ToForeground() {

services/connector/service_test.go

@@ -1,10 +1,13 @@
 package connector
 
 import (
+	"net"
+	"strconv"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestNewService(t *testing.T) {
@@ -57,20 +60,64 @@ func TestService_APIs(t *testing.T) {
 func TestService_PauseResumeBackground(t *testing.T) {
 	state := setupTests(t)
 
+	// Pause/resume only affects the WS listener path; use a pre-assigned port so we can dial it.
+	host, port := freeLocalTCPPort(t)
+	addr := net.JoinHostPort(host, strconv.Itoa(port))
+	state.service.config.WSEnabled = true
+	state.service.config.WSHost = host
+	state.service.config.WSPort = port
+
+	t.Cleanup(func() { _ = state.service.Stop() })
+
 	err := state.service.Start()
-	assert.NoError(t, err)
+	require.NoError(t, err)
+	require.NotNil(t, state.service.wsServer)
+	waitUntilTCPAccepts(t, addr)
 
 	err = state.service.PauseBackground()
-	assert.NoError(t, err)
-	assert.True(t, state.service.paused)
+	require.NoError(t, err)
+	require.True(t, state.service.paused)
+	waitUntilTCPRefused(t, addr)
 
 	err = state.service.ResumeForeground()
-	assert.NoError(t, err)
-	assert.False(t, state.service.paused)
-
-	// Give the listener goroutine a small window to bind.
-	time.Sleep(50 * time.Millisecond)
+	require.NoError(t, err)
+	require.False(t, state.service.paused)
+	waitUntilTCPAccepts(t, addr)
 
 	err = state.service.Stop()
-	assert.NoError(t, err)
+	require.NoError(t, err)
+}
+
+// freeLocalTCPPort reserves an ephemeral TCP port on loopback for the test process.
+// There is a small race where another process could bind between Close and Start; acceptable for tests.
+func freeLocalTCPPort(t *testing.T) (host string, port int) {
+	t.Helper()
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	tcpAddr := ln.Addr().(*net.TCPAddr)
+	require.NoError(t, ln.Close())
+	return tcpAddr.IP.String(), tcpAddr.Port
+}
+
+func waitUntilTCPAccepts(t *testing.T, addr string) {
+	t.Helper()
+	require.Eventually(t, func() bool {
+		c, err := net.DialTimeout("tcp", addr, 50*time.Millisecond)
+		if err != nil {
+			return false
+		}
+		_ = c.Close()
+		return true
+	}, 5*time.Second, 10*time.Millisecond, "expected TCP accept on %s", addr)
+}
+
+func waitUntilTCPRefused(t *testing.T, addr string) {
+	t.Helper()
+	require.Eventually(t, func() bool {
+		c, err := net.DialTimeout("tcp", addr, 50*time.Millisecond)
+		if c != nil {
+			_ = c.Close()
+		}
+		return err != nil
+	}, 5*time.Second, 10*time.Millisecond, "expected no listener on %s after pause", addr)
 }