Fixed client sequence number race

Fixed a race condition on the sequence number increment as well as some timing issues with the tests.
* Move the increment into handleCommandResponse.
* Increased the mock server test timeout
* Made keep-alive check configurable for tests.
* Decreased the time between mock server message sends.

client.go

@@ -26,6 +26,11 @@ const (
 	clientTimeout = 45 * time.Second
 )
 
+var (
+	// keepAliveCheck is the check interval for keepalives
+	keepAliveCheck = time.Second
+)
+
 // Client represents a BattlEye client.
 type Client struct {
 	conn       net.Conn
@@ -125,8 +130,6 @@ func (c *Client) Exec(cmd string) (string, error) {
 			}
 			return "", err
 		}
-		// Increment because we've got a response.
-		c.incr()
 		return resp, nil
 	}
 
@@ -193,7 +196,7 @@ func (c *Client) connect(addr, pwd string) (err error) {
 func (c *Client) keepConnectionAlive() {
 	defer c.wg.Done()
 
-	t := time.NewTicker(time.Second)
+	t := time.NewTicker(keepAliveCheck)
 	for {
 		select {
 		case <-c.done.C():
@@ -270,6 +273,7 @@ func (c *Client) handleCommandResponse(r *commandResponse) {
 
 	// response is not fragmented.
 	if !r.multi {
+		c.incr()
 		c.cmds <- r.msg
 		return
 	}
@@ -285,6 +289,7 @@ func (c *Client) handleCommandResponse(r *commandResponse) {
 
 	// If the message is complete send it.
 	if fr.completed() {
+		c.incr()
 		c.cmds <- fr.message()
 	}
 }

client_test.go

@@ -1,6 +1,7 @@
 package battleye
 
 import (
+	"fmt"
 	"testing"
 	"time"
 
@@ -17,6 +18,7 @@ func TestClient(t *testing.T) {
 		name           string
 		clientPassword string
 		clientOpts     []Option
+		keepAliveCheck time.Duration
 		expClientErr   error
 		closesClient   bool
 		testfunc       func(*testing.T, *Client, *server)
@@ -54,9 +56,10 @@ func TestClient(t *testing.T) {
 			},
 		},
 		{
-			name:         "Successful login, keep-alive and execute command",
-			clientOpts:   []Option{Timeout(testTimeout), KeepAlive(200 * time.Millisecond), MessageBuffer(10)},
-			closesClient: true,
+			name:           "Successful login, keep-alive and execute command",
+			clientOpts:     []Option{Timeout(testTimeout), KeepAlive(200 * time.Millisecond), MessageBuffer(10)},
+			closesClient:   true,
+			keepAliveCheck: time.Millisecond * 10,
 			testfunc: func(t *testing.T, c *Client, s *server) {
 				defer func() {
 					if !assert.NoError(t, c.Close()) {
@@ -64,7 +67,8 @@ func TestClient(t *testing.T) {
 					}
 
 					// mock server must have received some keep alive packets
-					assert.True(t, s.keepAlive() >= 1)
+					v := s.keepAlive()
+					assert.True(t, v >= 1, fmt.Sprintf("keep-alive: %v not greater than one", v))
 					// mock server must have received some server message acknowledge packets
 					assert.True(t, s.srvMsgAck() >= 1)
 					// we must have received at least 1 server message
@@ -116,6 +120,12 @@ func TestClient(t *testing.T) {
 				tc.clientPassword = testPassword
 			}
 
+			if tc.keepAliveCheck > 0 {
+				old := keepAliveCheck
+				keepAliveCheck = tc.keepAliveCheck
+				defer func() { keepAliveCheck = old }()
+			}
+
 			c, err := NewClient(s.Addr, tc.clientPassword, tc.clientOpts...)
 			defer func() {
 				if !tc.closesClient && c != nil {

mockserver_test.go

@@ -16,7 +16,7 @@ import (
 )
 
 const (
-	testTimeout       = 100 * time.Millisecond
+	testTimeout       = time.Second
 	testAddress       = "127.0.0.1:0"
 	testServerMessage = "server broadcast"
 )
@@ -138,11 +138,11 @@ func (s *server) serve() {
 	}
 }
 
-// messenger sends a server message per second to each known client.
+// messenger sends a server message to each known client regularly.
 func (s *server) messenger() {
 	defer s.wg.Done()
 
-	t := time.NewTicker(1 * time.Second)
+	t := time.NewTicker(time.Millisecond * 100)
 	defer t.Stop()
 
 	for {