From fc688ae15e1bfeca49c04371bb66ad3456f73a50 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 10 Oct 2024 17:32:07 +0200 Subject: [PATCH 1/4] Fixed linter warn --- internal/cli/daemon/daemon.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/cli/daemon/daemon.go b/internal/cli/daemon/daemon.go index 3139a66bdde..839738b8999 100644 --- a/internal/cli/daemon/daemon.go +++ b/internal/cli/daemon/daemon.go @@ -35,7 +35,6 @@ import ( ) var ( - tr = i18n.Tr daemonize bool debug bool debugFile string From 9bb46180a74256e5fc4a36b9d6f7140c17644ff8 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 10 Oct 2024 17:46:09 +0200 Subject: [PATCH 2/4] Added integration test --- .../daemon/daemon_concurrency_test.go | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/internal/integrationtest/daemon/daemon_concurrency_test.go b/internal/integrationtest/daemon/daemon_concurrency_test.go index 733fe54504a..4f826aa3147 100644 --- a/internal/integrationtest/daemon/daemon_concurrency_test.go +++ b/internal/integrationtest/daemon/daemon_concurrency_test.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "io" + "sync" "testing" "time" @@ -76,3 +77,51 @@ func TestArduinoCliDaemonCompileWithLotOfOutput(t *testing.T) { testCompile() testCompile() } + +func TestInitAndMonitorConcurrency(t *testing.T) { + // See: https://github.com/arduino/arduino-cli/issues/2719 + + env, cli := integrationtest.CreateEnvForDaemon(t) + defer env.CleanUp() + + _, _, err := cli.Run("core", "install", "arduino:avr") + require.NoError(t, err) + + grpcInst := cli.Create() + require.NoError(t, grpcInst.Init("", "", func(ir *commands.InitResponse) { + fmt.Printf("INIT> %v\n", ir.GetMessage()) + })) + + cli.InstallMockedSerialMonitor(t) + + // Open the serial monitor for 5 seconds + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + mon, err := grpcInst.Monitor(ctx, &commands.Port{ + Address: "/dev/test", + Protocol: "serial", + }) + require.NoError(t, err) + var monitorCompleted sync.WaitGroup + monitorCompleted.Add(1) + go func() { + for { + msg, err := mon.Recv() + if err != nil { + break + } + fmt.Println("MON> ", msg) + } + fmt.Println("MON CLOSED") + monitorCompleted.Done() + }() + + // Check that Init completes without blocking when the monitor is open + start := time.Now() + require.NoError(t, grpcInst.Init("", "", func(ir *commands.InitResponse) { + fmt.Printf("INIT> %v\n", ir.GetMessage()) + })) + require.LessOrEqual(t, time.Since(start), 4*time.Second) + cancel() + monitorCompleted.Wait() +} From 576835243b77e9d865e6bc1a4088d7d2cfd08a70 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 10 Oct 2024 17:51:37 +0200 Subject: [PATCH 3/4] Fixed monitor dead-locking package manager --- commands/service_monitor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands/service_monitor.go b/commands/service_monitor.go index 7c47e201ada..8c3402681b7 100644 --- a/commands/service_monitor.go +++ b/commands/service_monitor.go @@ -125,8 +125,8 @@ func (s *arduinoCoreServerImpl) Monitor(stream rpc.ArduinoCoreService_MonitorSer if err != nil { return err } - defer release() monitor, boardSettings, err := findMonitorAndSettingsForProtocolAndBoard(pme, openReq.GetPort().GetProtocol(), openReq.GetFqbn()) + release() if err != nil { return err } From a812e1aadbc12546ae862171eb19f77b766fb3f2 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 10 Oct 2024 18:38:38 +0200 Subject: [PATCH 4/4] Fix integration test: Allow some time for the mocked-monitor process to terminate otherwise the test will fail on Windows when the cleanup function tries to remove the executable: Error Trace: D:/a/arduino-cli/arduino-cli/internal/integrationtest/environment.go:46 D:/a/arduino-cli/arduino-cli/internal/integrationtest/environment.go:56 D:/a/arduino-cli/arduino-cli/internal/integrationtest/environment.go:56 D:/a/arduino-cli/arduino-cli/internal/integrationtest/environment.go:62 D:/a/arduino-cli/arduino-cli/internal/integrationtest/daemon/daemon_concurrency_test.go:127 Error: Received unexpected error: remove C:\Users\runneradmin\AppData\Local\Temp\cli2489057723\A\packages\builtin\tools\serial-monitor\0.14.1\serial-monitor.exe: Access is denied. Test: TestInitAndMonitorConcurrency --- internal/integrationtest/daemon/daemon_concurrency_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/integrationtest/daemon/daemon_concurrency_test.go b/internal/integrationtest/daemon/daemon_concurrency_test.go index 4f826aa3147..c3541efbf4d 100644 --- a/internal/integrationtest/daemon/daemon_concurrency_test.go +++ b/internal/integrationtest/daemon/daemon_concurrency_test.go @@ -124,4 +124,9 @@ func TestInitAndMonitorConcurrency(t *testing.T) { require.LessOrEqual(t, time.Since(start), 4*time.Second) cancel() monitorCompleted.Wait() + + // Allow some time for the mocked-monitor process to terminate (otherwise the + // test will fail on Windows when the cleanup function tries to remove the + // executable). + time.Sleep(2 * time.Second) }