[FIXED] LeafNode: data race during validation and create leafnode (#4194

)

The issue really was that the test was sharing remote options. The way
options are used is not ideal since we reference the user provided
options (but it is relied upon now in many tests and possibly users
setups). The other side of the issue was that when no local account is
specified in a "remote" specification, we set it to the global account,
but that was done when creating the leafnode object (when soliciting),
which in the case of the test could race with the second server doing
the validation.

In this PR we move the setting to global account during the validation,
but also fixed the tests to not share the remote options configuration
slice between the two servers.

Resolves #4191

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>

server/leafnode.go

@@ -191,6 +191,13 @@ func validateLeafNode(o *Options) error {
 		return err
 	}
 
+	// Users can bind to any local account, if its empty we will assume the $G account.
+	for _, r := range o.LeafNode.Remotes {
+		if r.LocalAccount == _EMPTY_ {
+			r.LocalAccount = globalAccountName
+		}
+	}
+
 	// In local config mode, check that leafnode configuration refers to accounts that exist.
 	if len(o.TrustedOperators) == 0 {
 		accNames := map[string]struct{}{}
@@ -933,15 +940,7 @@ func (s *Server) createLeafNode(conn net.Conn, rURL *url.URL, remote *leafNodeCf
 	if remote != nil {
 		// For now, if lookup fails, we will constantly try
 		// to recreate this LN connection.
-		remote.Lock()
-		// Users can bind to any local account, if its empty
-		// we will assume the $G account.
-		if remote.LocalAccount == _EMPTY_ {
-			remote.LocalAccount = globalAccountName
-		}
 		lacc := remote.LocalAccount
-		remote.Unlock()
-
 		var err error
 		acc, err = s.LookupAccount(lacc)
 		if err != nil {

server/leafnode_test.go

@@ -950,7 +950,7 @@ func TestLeafNodeLoopFromDAG(t *testing.T) {
 	checkLeafNodeConnectedCount(t, sc, 1)
 }
 
-func TestLeafCloseTLSConnection(t *testing.T) {
+func TestLeafNodeCloseTLSConnection(t *testing.T) {
 	opts := DefaultOptions()
 	opts.DisableShortFirstPing = true
 	opts.LeafNode.Host = "127.0.0.1"
@@ -2374,17 +2374,16 @@ func TestLeafNodeLMsgSplit(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Error parsing url: %v", err)
 	}
-	remoteLeafs := []*RemoteLeafOpts{{URLs: []*url.URL{u1, u2}}}
 
 	oLeaf1 := DefaultOptions()
 	oLeaf1.Cluster.Name = "xyz"
-	oLeaf1.LeafNode.Remotes = remoteLeafs
+	oLeaf1.LeafNode.Remotes = []*RemoteLeafOpts{{URLs: []*url.URL{u1, u2}}}
 	leaf1 := RunServer(oLeaf1)
 	defer leaf1.Shutdown()
 
 	oLeaf2 := DefaultOptions()
 	oLeaf2.Cluster.Name = "xyz"
-	oLeaf2.LeafNode.Remotes = remoteLeafs
+	oLeaf2.LeafNode.Remotes = []*RemoteLeafOpts{{URLs: []*url.URL{u1, u2}}}
 	oLeaf2.Routes = RoutesFromStr(fmt.Sprintf("nats://127.0.0.1:%d", oLeaf1.Cluster.Port))
 	leaf2 := RunServer(oLeaf2)
 	defer leaf2.Shutdown()
@@ -2473,11 +2472,10 @@ func TestLeafNodeRouteParseLSUnsub(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Error parsing url: %v", err)
 	}
-	remoteLeafs := []*RemoteLeafOpts{{URLs: []*url.URL{u2}}}
 
 	oLeaf2 := DefaultOptions()
 	oLeaf2.Cluster.Name = "xyz"
-	oLeaf2.LeafNode.Remotes = remoteLeafs
+	oLeaf2.LeafNode.Remotes = []*RemoteLeafOpts{{URLs: []*url.URL{u2}}}
 	leaf2 := RunServer(oLeaf2)
 	defer leaf2.Shutdown()