Fix gRPC BoardListWatch crashing when installing/uninstalling a core/lib

Author: silvanocerza

arduino/discovery/discovery.go

@@ -168,13 +168,19 @@ func (disc *PluggableDiscovery) jsonDecodeLoop(in io.Reader, outChan chan<- *dis
 		disc.incomingMessagesError = err
 		disc.statusMutex.Unlock()
 		close(outChan)
-		logrus.Errorf("stopped discovery %s decode loop", disc.id)
-		// TODO: Try restarting process some times before closing it completely
+		logrus.Errorf("stopped discovery %s decode loop: %v", disc.id, err)
 	}
 
 	for {
 		var msg discoveryMessage
-		if err := decoder.Decode(&msg); err != nil {
+		if err := decoder.Decode(&msg); err == io.EOF {
+			// This is fine, we exit gracefully
+			disc.statusMutex.Lock()
+			disc.state = Dead
+			disc.statusMutex.Unlock()
+			close(outChan)
+			return
+		} else if err != nil {
 			closeAndReportError(err)
 			return
 		}
@@ -218,9 +224,6 @@ func (disc *PluggableDiscovery) waitMessage(timeout time.Duration) (*discoveryMe
 	select {
 	case msg := <-disc.incomingMessagesChan:
 		if msg == nil {
-			// channel has been closed
-			disc.statusMutex.Lock()
-			defer disc.statusMutex.Unlock()
 			return nil, disc.incomingMessagesError
 		}
 		return msg, nil

arduino/discovery/discoverymanager/discoverymanager.go

@@ -192,6 +192,9 @@ func (dm *DiscoveryManager) QuitAll() []error {
 	// Close the global channel only if there were no errors
 	// quitting all alive discoveries
 	if len(errs) == 0 && dm.globalEventCh != nil {
+		// Let events consumers that discoveries are quitting no more events
+		// will be sent on this channel
+		dm.globalEventCh <- &discovery.Event{Type: "quit"}
 		close(dm.globalEventCh)
 		dm.globalEventCh = nil
 	}

commands/board/list.go

@@ -258,6 +258,25 @@ func Watch(instanceID int32, interrupt <-chan bool) (<-chan *rpc.BoardListWatchR
 		for {
 			select {
 			case event := <-eventsChan:
+				if event.Type == "quit" {
+					// The discovery manager has closed its event channel because it's
+					// quitting all the discovery processes that are running, this
+					// means that the events channel we're listening from won't receive any
+					// more events.
+					// Handling this case is necessary when the board watcher is running and
+					// the instance being used is reinitialized since that quits all the
+					// discovery processes and reset the discovery manager. That would leave
+					// this goroutine listening forever on a "dead" channel and might even
+					// cause panics.
+					// This message avoid all this issues.
+					// It will be the client's task restarting the board watcher if necessary,
+					// this host won't attempt restarting it.
+					outChan <- &rpc.BoardListWatchResponse{
+						EventType: event.Type,
+					}
+					return
+				}
+
 				port := &rpc.DetectedPort{
 					Port: event.Port.ToRPC(),
 				}
@@ -276,11 +295,11 @@ func Watch(instanceID int32, interrupt <-chan bool) (<-chan *rpc.BoardListWatchR
 					Error:     boardsError,
 				}
 			case <-interrupt:
-				err := pm.DiscoveryManager().StopAll()
-				if err != nil {
+				errs := dm.StopAll()
+				if len(errs) > 0 {
 					outChan <- &rpc.BoardListWatchResponse{
 						EventType: "error",
-						Error:     tr("stopping discoveries: %s", err),
+						Error:     tr("stopping discoveries: %s", errs),
 					}
 					// Don't close the channel if quitting all discoveries
 					// failed, otherwise some processes might be left running.

commands/daemon/daemon.go

@@ -31,6 +31,8 @@ import (
 	"github.com/arduino/arduino-cli/i18n"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/sirupsen/logrus"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 // ArduinoCoreServerImpl FIXMEDOC
@@ -109,6 +111,9 @@ func (s *ArduinoCoreServerImpl) BoardListWatch(stream rpc.ArduinoCoreService_Boa
 				logrus.Info("boards watcher stream closed")
 				interrupt <- true
 				return
+			} else if st, ok := status.FromError(err); ok && st.Code() == codes.Canceled {
+				logrus.Info("boards watcher interrupted by host")
+				return
 			} else if err != nil {
 				logrus.Infof("interrupting boards watcher: %v", err)
 				interrupt <- true