Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit ea09108

Browse files
authored
Fix monitor init handling (#2728)
* Fixed linter warn * Added integration test * Fixed monitor dead-locking package manager * 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-monitor0円.14.1\serial-monitor.exe: Access is denied. Test: TestInitAndMonitorConcurrency
1 parent ac6ec6d commit ea09108

File tree

3 files changed

+55
-2
lines changed

3 files changed

+55
-2
lines changed

‎commands/service_monitor.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,8 @@ func (s *arduinoCoreServerImpl) Monitor(stream rpc.ArduinoCoreService_MonitorSer
125125
if err != nil {
126126
return err
127127
}
128-
defer release()
129128
monitor, boardSettings, err := findMonitorAndSettingsForProtocolAndBoard(pme, openReq.GetPort().GetProtocol(), openReq.GetFqbn())
129+
release()
130130
if err != nil {
131131
return err
132132
}

‎internal/cli/daemon/daemon.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ import (
3535
)
3636

3737
var (
38-
tr = i18n.Tr
3938
daemonize bool
4039
debug bool
4140
debugFile string

‎internal/integrationtest/daemon/daemon_concurrency_test.go‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"errors"
2121
"fmt"
2222
"io"
23+
"sync"
2324
"testing"
2425
"time"
2526

@@ -76,3 +77,56 @@ func TestArduinoCliDaemonCompileWithLotOfOutput(t *testing.T) {
7677
testCompile()
7778
testCompile()
7879
}
80+
81+
func TestInitAndMonitorConcurrency(t *testing.T) {
82+
// See: https://github.com/arduino/arduino-cli/issues/2719
83+
84+
env, cli := integrationtest.CreateEnvForDaemon(t)
85+
defer env.CleanUp()
86+
87+
_, _, err := cli.Run("core", "install", "arduino:avr")
88+
require.NoError(t, err)
89+
90+
grpcInst := cli.Create()
91+
require.NoError(t, grpcInst.Init("", "", func(ir *commands.InitResponse) {
92+
fmt.Printf("INIT> %v\n", ir.GetMessage())
93+
}))
94+
95+
cli.InstallMockedSerialMonitor(t)
96+
97+
// Open the serial monitor for 5 seconds
98+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
99+
defer cancel()
100+
mon, err := grpcInst.Monitor(ctx, &commands.Port{
101+
Address: "/dev/test",
102+
Protocol: "serial",
103+
})
104+
require.NoError(t, err)
105+
var monitorCompleted sync.WaitGroup
106+
monitorCompleted.Add(1)
107+
go func() {
108+
for {
109+
msg, err := mon.Recv()
110+
if err != nil {
111+
break
112+
}
113+
fmt.Println("MON> ", msg)
114+
}
115+
fmt.Println("MON CLOSED")
116+
monitorCompleted.Done()
117+
}()
118+
119+
// Check that Init completes without blocking when the monitor is open
120+
start := time.Now()
121+
require.NoError(t, grpcInst.Init("", "", func(ir *commands.InitResponse) {
122+
fmt.Printf("INIT> %v\n", ir.GetMessage())
123+
}))
124+
require.LessOrEqual(t, time.Since(start), 4*time.Second)
125+
cancel()
126+
monitorCompleted.Wait()
127+
128+
// Allow some time for the mocked-monitor process to terminate (otherwise the
129+
// test will fail on Windows when the cleanup function tries to remove the
130+
// executable).
131+
time.Sleep(2 * time.Second)
132+
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /