Merge pull request #58 from mendersoftware/cherry-1.2.x-shells_spawne…

…d_not_decreased_in_all_cases

[Cherry 1.2.x]: fix: Global spawned shells count is not always decremented.

app/daemon.go

@@ -1,4 +1,4 @@
-// Copyright 2021 Northern.tech AS
+// Copyright 2022 Northern.tech AS
 //
 //    Licensed under the Apache License, Version 2.0 (the "License");
 //    you may not use this file except in compliance with the License.
@@ -55,6 +55,7 @@ type MenderShellDaemon struct {
 	ctx                     context.Context
 	ctxCancel               context.CancelFunc
 	writeMutex              *sync.Mutex
+	spawnedShellsMutex      *sync.Mutex
 	eventChan               chan MenderShellDaemonEvent
 	connectionEstChan       chan MenderShellDaemonEvent
 	reconnectChan           chan MenderShellDaemonEvent
@@ -114,6 +115,7 @@ func NewDaemon(conf *config.MenderShellConfig) *MenderShellDaemon {
 		ctx:                     ctx,
 		ctxCancel:               ctxCancel,
 		writeMutex:              &sync.Mutex{},
+		spawnedShellsMutex:      &sync.Mutex{},
 		eventChan:               make(chan MenderShellDaemonEvent),
 		connectionEstChan:       make(chan MenderShellDaemonEvent),
 		reconnectChan:           make(chan MenderShellDaemonEvent),
@@ -195,6 +197,9 @@ func (d *MenderShellDaemon) wsReconnect(token string) (err error) {
 func (d *MenderShellDaemon) outputStatus() {
 	log.Infof("mender-connect daemon v%s", config.VersionString())
 	log.Info(" status: ")
+	d.spawnedShellsMutex.Lock()
+	log.Infof("  shells: %d/%d", d.shellsSpawned, config.MaxShellsSpawned)
+	d.spawnedShellsMutex.Unlock()
 	log.Infof("  sessions: %d", session.MenderShellSessionGetCount())
 	sessionIds := session.MenderShellSessionGetSessionIds()
 	for _, id := range sessionIds {
@@ -299,6 +304,9 @@ func (d *MenderShellDaemon) processJwtTokenStateChange(jwtToken, serverUrl strin
 				log.Errorf("dbusEventLoop error terminating all sessions: %s",
 					err.Error())
 			}
+			if shellsCount > 0 {
+				d.DecreaseSpawnedShellsCount(uint(shellsCount))
+			}
 		}
 		connectionmanager.Close(ws.ProtoTypeShell)
 		d.authorized = false
@@ -426,6 +434,20 @@ func (d *MenderShellDaemon) setupLogging() {
 	}
 }
 
+func (d *MenderShellDaemon) DecreaseSpawnedShellsCount(shellStoppedCount uint) {
+	d.spawnedShellsMutex.Lock()
+	defer d.spawnedShellsMutex.Unlock()
+	if d.shellsSpawned == 0 {
+		log.Warn("can't decrement shellsSpawned count: it is 0.")
+	} else {
+		if shellStoppedCount >= d.shellsSpawned {
+			d.shellsSpawned = 0
+		} else {
+			d.shellsSpawned -= shellStoppedCount
+		}
+	}
+}
+
 //starts all needed elements of the mender-connect daemon
 // * executes given shell (shell.ExecuteShell)
 // * get dbus API and starts the dbus main loop (dbus.GetDBusAPI(), go dbusAPI.MainLoopRun(loop))
@@ -512,7 +534,8 @@ func (d *MenderShellDaemon) Run() error {
 			if err != nil {
 				log.Errorf("main-loop: failed to terminate some expired sessions, left: %d",
 					totalExpiredLeft)
-			} else if sessionStoppedCount != 0 {
+			} else if sessionStoppedCount > 0 {
+				d.DecreaseSpawnedShellsCount(uint(sessionStoppedCount))
 				log.Infof("main-loop: stopped %d sessions, %d shells, expired sessions left: %d",
 					shellStoppedCount, sessionStoppedCount, totalExpiredLeft)
 			}

app/daemon_shell.go

@@ -1,4 +1,4 @@
-// Copyright 2021 Northern.tech AS
+// Copyright 2022 Northern.tech AS
 //
 //    Licensed under the Apache License, Version 2.0 (the "License");
 //    you may not use this file except in compliance with the License.
@@ -133,7 +133,7 @@ func (d *MenderShellDaemon) routeMessageStopShell(message *ws.ProtoMsg) error {
 				return err
 			} else {
 				log.Debugf("StopByUserId: stopped %d shells.", shellsStoppedCount)
-				d.shellsSpawned -= shellsStoppedCount
+				d.DecreaseSpawnedShellsCount(shellsStoppedCount)
 			}
 		}
 		d.routeMessageResponse(response, err)

app/daemon_test.go

@@ -1,4 +1,4 @@
-// Copyright 2021 Northern.tech AS
+// Copyright 2022 Northern.tech AS
 //
 //    Licensed under the Apache License, Version 2.0 (the "License");
 //    you may not use this file except in compliance with the License.
@@ -1175,3 +1175,75 @@ func TestRouteMessage(t *testing.T) {
 		})
 	}
 }
+
+func TestDecreaseSpawnedShellsCount(t *testing.T) {
+	t.Parallel()
+	testCases := []struct {
+		Name string
+
+		CurrentCount  uint
+		DecreaseBy    uint
+		ExpectedCount uint
+	}{
+		{
+			Name: "decrease by 1 with 1",
+
+			CurrentCount: 1,
+			DecreaseBy:   1,
+		},
+		{
+			Name: "decrease by 1 with many",
+
+			CurrentCount:  3,
+			DecreaseBy:    1,
+			ExpectedCount: 2,
+		},
+		{
+			Name: "decrease by many with many",
+
+			CurrentCount: 3,
+			DecreaseBy:   3,
+		},
+		{
+			Name: "decrease by many with some",
+
+			CurrentCount:  255,
+			DecreaseBy:    3,
+			ExpectedCount: 252,
+		},
+		{
+			Name: "decrease by some with many",
+
+			CurrentCount:  3,
+			DecreaseBy:    255,
+			ExpectedCount: 0,
+		},
+		{
+			Name: "decrease by many with 0",
+
+			DecreaseBy: 3,
+		},
+		{
+			Name: "decrease by 0 with 0",
+		},
+	}
+	for i := range testCases {
+		tc := testCases[i]
+		t.Run(tc.Name, func(t *testing.T) {
+			t.Parallel()
+
+			daemon := NewDaemon(&config.MenderShellConfig{
+				MenderShellConfigFromFile: config.MenderShellConfigFromFile{
+					FileTransfer: config.FileTransferConfig{
+						Disable: false,
+					},
+				},
+			})
+
+			daemon.shellsSpawned = tc.CurrentCount
+			daemon.DecreaseSpawnedShellsCount(tc.DecreaseBy)
+
+			assert.Equal(t, tc.ExpectedCount, daemon.shellsSpawned)
+		})
+	}
+}