fix: Multiple gRPC Create/Init/Destroy cycles causes multiple pluggable-discovery zombie process (#2985)

* Refactor mocked-discovery integration tests

* Added integration-test to check handling of multiple discovery instances

* Correctly clean-up pluggable-discoveries on gRPC instance close

Author: cmaglie

commands/internal/instances/instances.go

@@ -172,10 +172,12 @@ func IsValid(inst *rpc.Instance) bool {
 // Delete removes an instance.
 func Delete(inst *rpc.Instance) bool {
 	instancesMux.Lock()
-	defer instancesMux.Unlock()
-	if _, ok := instances[inst.GetId()]; !ok {
+	instance, ok := instances[inst.GetId()]
+	delete(instances, inst.GetId())
+	instancesMux.Unlock()
+	if !ok {
 		return false
 	}
-	delete(instances, inst.GetId())
+	instance.pm.Destroy()
 	return true
 }

internal/arduino/cores/packagemanager/package_manager.go

@@ -197,6 +197,11 @@ func (pm *PackageManager) NewExplorer() (explorer *Explorer, release func()) {
 	}, pm.packagesLock.RUnlock
 }
 
+// Destroy releases all resources held by the PackageManager.
+func (pm *PackageManager) Destroy() {
+	pm.discoveryManager.Clear()
+}
+
 // GetProfile returns the active profile for this package manager, or nil if no profile is selected.
 func (pme *Explorer) GetProfile() *sketch.Profile {
 	return pme.profile

internal/integrationtest/arduino-cli.go

@@ -506,6 +506,13 @@ func (cli *ArduinoCLI) Create() *ArduinoCLIInstance {
 	}
 }
 
+// Destroy calls the "Destroy" gRPC method.
+func (inst *ArduinoCLIInstance) Destroy(ctx context.Context) error {
+	logCallf(">>> Destroy(%v)\n", inst.instance.GetId())
+	_, err := inst.cli.daemonClient.Destroy(ctx, &commands.DestroyRequest{Instance: inst.instance})
+	return err
+}
+
 // SetValue calls the "SetValue" gRPC method.
 func (cli *ArduinoCLI) SetValue(key, jsonData string) error {
 	req := &commands.SettingsSetValueRequest{

internal/integrationtest/board/board_test.go

@@ -71,23 +71,6 @@ func TestCorrectBoardListOrdering(t *testing.T) {
 	]`)
 }
 
-func TestBoardList(t *testing.T) {
-	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
-	defer env.CleanUp()
-
-	_, _, err := cli.Run("core", "update-index")
-	require.NoError(t, err)
-
-	cli.InstallMockedSerialDiscovery(t)
-
-	stdout, _, err := cli.Run("board", "list", "--json")
-	require.NoError(t, err)
-	// check is a valid json and contains a list of ports
-	requirejson.Parse(t, stdout).
-		Query(`[ .detected_ports | .[].port | select(.protocol == null or .protocol_label == null) ]`).
-		MustBeEmpty()
-}
-
 func TestBoardListMock(t *testing.T) {
 	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
 	defer env.CleanUp()
@@ -97,11 +80,20 @@ func TestBoardListMock(t *testing.T) {
 
 	cli.InstallMockedSerialDiscovery(t)
 
-	stdout, _, err := cli.Run("board", "list", "--json")
-	require.NoError(t, err)
+	t.Run("IsValidJsonPortList", func(t *testing.T) {
+		// 1. Check is a valid JSON
+		// 2. Check is a valid port list (has address/protocol)
+		stdout, _, err := cli.Run("board", "list", "--json")
+		require.NoError(t, err)
+		requirejson.Parse(t, stdout).
+			Query(`[ .detected_ports | .[].port | select(.address == null or .protocol == null) ]`).
+			MustBeEmpty()
+	})
 
-	// check is a valid json and contains a list of ports
-	requirejson.Contains(t, stdout, `{
+	t.Run("ContainsMockedPorts", func(t *testing.T) {
+		stdout, _, err := cli.Run("board", "list", "--json")
+		require.NoError(t, err)
+		requirejson.Contains(t, stdout, `{
 		  "detected_ports": [
 			{
 			  "matching_boards": [
@@ -124,35 +116,20 @@ func TestBoardListMock(t *testing.T) {
 			  }
 			}
 		  ]
-	}`)
-}
-
-func TestBoardListWithFqbnFilter(t *testing.T) {
-	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
-	defer env.CleanUp()
-
-	_, _, err := cli.Run("core", "update-index")
-	require.NoError(t, err)
-
-	cli.InstallMockedSerialDiscovery(t)
-
-	stdout, _, err := cli.Run("board", "list", "-b", "foo:bar:baz", "--json")
-	require.NoError(t, err)
-	requirejson.Query(t, stdout, `.detected_ports | length`, `0`)
-}
-
-func TestBoardListWithFqbnFilterInvalid(t *testing.T) {
-	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
-	defer env.CleanUp()
-
-	_, _, err := cli.Run("core", "update-index")
-	require.NoError(t, err)
+		}`)
+	})
 
-	cli.InstallMockedSerialDiscovery(t)
+	t.Run("ListWithFqbnFilter", func(t *testing.T) {
+		stdout, _, err := cli.Run("board", "list", "-b", "foo:bar:baz", "--json")
+		require.NoError(t, err)
+		requirejson.Query(t, stdout, `.detected_ports | length`, `0`)
+	})
 
-	_, stderr, err := cli.Run("board", "list", "-b", "yadayada", "--json")
-	require.Error(t, err)
-	requirejson.Query(t, stderr, ".error", `"Invalid FQBN: not an FQBN: yadayada"`)
+	t.Run("ListWithFqbnFilterInvalid", func(t *testing.T) {
+		_, stderr, err := cli.Run("board", "list", "-b", "yadayada", "--json")
+		require.Error(t, err)
+		requirejson.Query(t, stderr, ".error", `"Invalid FQBN: not an FQBN: yadayada"`)
+	})
 }
 
 func TestBoardListall(t *testing.T) {

internal/integrationtest/daemon/discoveries_test.go

@@ -0,0 +1,86 @@
+// This file is part of arduino-cli.
+//
+// Copyright 2025 ARDUINO SA (http://www.arduino.cc/)
+//
+// This software is released under the GNU General Public License version 3,
+// which covers the main part of arduino-cli.
+// The terms of this license can be found at:
+// https://www.gnu.org/licenses/gpl-3.0.en.html
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to
+// modify or otherwise use the software for commercial activities involving the
+// Arduino software without disclosing the source code of your own applications.
+// To purchase a commercial license, send an email to [email protected].
+
+package daemon
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/arduino/arduino-cli/internal/integrationtest"
+	"github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
+	"github.com/stretchr/testify/require"
+)
+
+func TestBoardListMock(t *testing.T) {
+	env, cli := integrationtest.CreateEnvForDaemon(t)
+	defer env.CleanUp()
+
+	_, _, err := cli.Run("core", "update-index")
+	require.NoError(t, err)
+
+	cli.InstallMockedSerialDiscovery(t)
+
+	var tmp1, tmp2 string
+
+	{
+		// Create a new instance of the daemon
+		grpcInst := cli.Create()
+		require.NoError(t, grpcInst.Init("", "", func(ir *commands.InitResponse) {
+			fmt.Printf("INIT> %v\n", ir.GetMessage())
+		}))
+
+		// Run a BoardList
+		resp, err := grpcInst.BoardList(time.Second)
+		require.NoError(t, err)
+		require.NotEmpty(t, resp.Ports)
+		for _, port := range resp.Ports {
+			if port.GetPort().GetProtocol() == "serial" {
+				tmp1 = port.Port.GetProperties()["discovery_tmp"]
+			}
+		}
+		require.NotEmpty(t, tmp1)
+
+		// Close instance
+		require.NoError(t, grpcInst.Destroy(t.Context()))
+	}
+
+	{
+		// Create a second instance of the daemon
+		grpcInst := cli.Create()
+		require.NoError(t, grpcInst.Init("", "", func(ir *commands.InitResponse) {
+			fmt.Printf("INIT> %v\n", ir.GetMessage())
+		}))
+
+		// Run a BoardList
+		resp, err := grpcInst.BoardList(time.Second)
+		require.NoError(t, err)
+		require.NotEmpty(t, resp.Ports)
+		for _, port := range resp.Ports {
+			if port.GetPort().GetProtocol() == "serial" {
+				tmp2 = port.Port.GetProperties()["discovery_tmp"]
+			}
+		}
+		require.NotEmpty(t, tmp2)
+
+		// Close instance
+		require.NoError(t, grpcInst.Destroy(t.Context()))
+	}
+
+	// Check if the discoveries have been successfully close
+	require.NoFileExists(t, tmp1, "discovery has not been closed")
+	require.NoFileExists(t, tmp2, "discovery has not been closed")
+}

internal/mock_serial_discovery/main.go

@@ -19,20 +19,39 @@ package main
 
 import (
 	"errors"
+	"fmt"
 	"os"
 	"time"
 
+	"github.com/arduino/go-paths-helper"
 	"github.com/arduino/go-properties-orderedmap"
 	discovery "github.com/arduino/pluggable-discovery-protocol-handler/v2"
 )
 
 type mockSerialDiscovery struct {
 	startSyncCount int
 	closeChan      chan<- bool
+	tmpFile        string
 }
 
 func main() {
-	dummy := &mockSerialDiscovery{}
+	// Write a file in a $TMP/mock_serial_discovery folder.
+	// This file will be used by the integration tests to detect if the discovery is running.
+	tmpDir := paths.TempDir().Join("mock_serial_discovery")
+	if err := tmpDir.MkdirAll(); err != nil {
+		fmt.Fprintf(os.Stderr, "Error creating temp dir: %v\n", err)
+		os.Exit(1)
+	}
+	tmpFile, err := paths.MkTempFile(tmpDir, "")
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "Error creating temp file: %v\n", err)
+		os.Exit(1)
+	}
+	tmpFile.Close()
+	defer os.Remove(tmpFile.Name())
+
+	// Run mock discovery
+	dummy := &mockSerialDiscovery{tmpFile: tmpFile.Name()}
 	server := discovery.NewServer(dummy)
 	if err := server.Run(os.Stdin, os.Stdout); err != nil {
 		os.Exit(1)
@@ -80,9 +99,10 @@ func (d *mockSerialDiscovery) StartSync(eventCB discovery.EventCallback, errorCB
 			ProtocolLabel: "Serial",
 			HardwareID:    "123456",
 			Properties: properties.NewFromHashmap(map[string]string{
-				"vid":    "0x2341",
-				"pid":    "0x0041",
-				"serial": "123456",
+				"vid":           "0x2341",
+				"pid":           "0x0041",
+				"serial":        "123456",
+				"discovery_tmp": d.tmpFile,
 			}),
 		})