fix: handle a failed Start() safely at the node and library layers (#1312)

Signed-off-by: kblinichkin <kirill.blinichkin@gmail.com>

library/node.go

@@ -261,12 +261,17 @@ func Start(instance *WakuInstance) error {
 		return err
 	}
 
-	instance.ctx, instance.cancel = context.WithCancel(context.Background())
+	ctx, cancel := context.WithCancel(context.Background())
 
-	if err := instance.node.Start(instance.ctx); err != nil {
+	if err := instance.node.Start(ctx); err != nil {
+		cancel()
 		return err
 	}
 
+	// Record started state only after node.Start() succeeds, so a failed Start()
+	// leaves IsStarted() false and Free() will not Stop() a node that never ran.
+	instance.ctx, instance.cancel = ctx, cancel
+
 	if instance.node.DiscV5() != nil {
 		if err := instance.node.DiscV5().Start(context.Background()); err != nil {
 			stop(instance)

library/node_start_test.go

@@ -0,0 +1,27 @@
+package library
+
+import (
+	"fmt"
+	"net"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// TestStartFailureLeavesInstanceStopped checks that a failed Start() leaves the
+// instance not-started and Free() safe (here the listen port is already in use).
+func TestStartFailureLeavesInstanceStopped(t *testing.T) {
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	defer ln.Close()
+	addr := ln.Addr().(*net.TCPAddr)
+
+	instance := Init()
+	cfg := fmt.Sprintf(`{"host":"127.0.0.1","port":%d,"relay":false,"store":false,"discV5":false}`, addr.Port)
+	require.NoError(t, NewNode(instance, cfg))
+
+	err = Start(instance)
+	require.Error(t, err, "Start should fail when the port is in use")
+	require.False(t, IsStarted(instance), "instance must not report started after a failed Start")
+	require.NotPanics(t, func() { _ = Free(instance) })
+}

waku/v2/node/stop_test.go

@@ -0,0 +1,34 @@
+package node
+
+import (
+	"context"
+	"net"
+	"testing"
+
+	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/stretchr/testify/require"
+	"github.com/waku-org/go-waku/tests"
+)
+
+// TestStopAfterFailedStart checks that Stop() is safe when Start() failed before
+// the host was created (here the listen port is already in use).
+func TestStopAfterFailedStart(t *testing.T) {
+	// Occupy a TCP port so libp2p host creation fails to listen on it.
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	defer ln.Close()
+	addr := ln.Addr().(*net.TCPAddr)
+
+	key, err := tests.RandomHex(32)
+	require.NoError(t, err)
+	prvKey, err := crypto.HexToECDSA(key)
+	require.NoError(t, err)
+
+	wakuNode, err := New(WithPrivateKey(prvKey), WithHostAddress(addr))
+	require.NoError(t, err)
+
+	err = wakuNode.Start(context.Background())
+	require.Error(t, err, "Start should fail when the port is in use")
+
+	require.NotPanics(t, func() { wakuNode.Stop() })
+}

waku/v2/node/wakunode2.go

@@ -533,6 +533,12 @@ func (w *WakuNode) Stop() {
 
 	w.stopWithTimeout(w.bcaster.Stop, "bcaster", 5*time.Second)
 
+	// Start() can fail before the host is created (e.g. port in use), leaving a
+	// partially-started node; guard so Stop() cannot nil-dereference w.host.
+	if w.host == nil {
+		return
+	}
+
 	defer w.connectionNotif.Close()
 	defer w.addressChangesSub.Close()