Movatterモバイル変換

[0]ホーム
URL:

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

Commit08ff7e2

Browse files
authored
fix: Multiple gRPCCreate/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
1 parent869fced commit08ff7e2

File tree

6 files changed

+152
-55
lines changed

6 files changed

+152
-55
lines changed

‎commands/internal/instances/instances.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,10 +172,12 @@ func IsValid(inst *rpc.Instance) bool {
172172
// Delete removes an instance.
173173
funcDelete(inst*rpc.Instance)bool {
174174
instancesMux.Lock()
175-
deferinstancesMux.Unlock()
176-
if_,ok:=instances[inst.GetId()];!ok {
175+
instance,ok:=instances[inst.GetId()]
176+
delete(instances,inst.GetId())
177+
instancesMux.Unlock()
178+
if!ok {
177179
returnfalse
178180
}
179-
delete(instances,inst.GetId())
181+
instance.pm.Destroy()
180182
returntrue
181183
}

‎internal/arduino/cores/packagemanager/package_manager.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,11 @@ func (pm *PackageManager) NewExplorer() (explorer *Explorer, release func()) {
197197
},pm.packagesLock.RUnlock
198198
}
199199

200+
// Destroy releases all resources held by the PackageManager.
201+
func (pm*PackageManager)Destroy() {
202+
pm.discoveryManager.Clear()
203+
}
204+
200205
// GetProfile returns the active profile for this package manager, or nil if no profile is selected.
201206
func (pme*Explorer)GetProfile()*sketch.Profile {
202207
returnpme.profile

‎internal/integrationtest/arduino-cli.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,13 @@ func (cli *ArduinoCLI) Create() *ArduinoCLIInstance {
506506
}
507507
}
508508

509+
// Destroy calls the "Destroy" gRPC method.
510+
func (inst*ArduinoCLIInstance)Destroy(ctx context.Context)error {
511+
logCallf(">>> Destroy(%v)\n",inst.instance.GetId())
512+
_,err:=inst.cli.daemonClient.Destroy(ctx,&commands.DestroyRequest{Instance:inst.instance})
513+
returnerr
514+
}
515+
509516
// SetValue calls the "SetValue" gRPC method.
510517
func (cli*ArduinoCLI)SetValue(key,jsonDatastring)error {
511518
req:=&commands.SettingsSetValueRequest{

‎internal/integrationtest/board/board_test.go‎

Lines changed: 25 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -71,23 +71,6 @@ func TestCorrectBoardListOrdering(t *testing.T) {
7171
]`)
7272
}
7373

74-
funcTestBoardList(t*testing.T) {
75-
env,cli:=integrationtest.CreateArduinoCLIWithEnvironment(t)
76-
deferenv.CleanUp()
77-
78-
_,_,err:=cli.Run("core","update-index")
79-
require.NoError(t,err)
80-
81-
cli.InstallMockedSerialDiscovery(t)
82-
83-
stdout,_,err:=cli.Run("board","list","--json")
84-
require.NoError(t,err)
85-
// check is a valid json and contains a list of ports
86-
requirejson.Parse(t,stdout).
87-
Query(`[ .detected_ports | .[].port | select(.protocol == null or .protocol_label == null) ]`).
88-
MustBeEmpty()
89-
}
90-
9174
funcTestBoardListMock(t*testing.T) {
9275
env,cli:=integrationtest.CreateArduinoCLIWithEnvironment(t)
9376
deferenv.CleanUp()
@@ -97,11 +80,20 @@ func TestBoardListMock(t *testing.T) {
9780

9881
cli.InstallMockedSerialDiscovery(t)
9982

100-
stdout,_,err:=cli.Run("board","list","--json")
101-
require.NoError(t,err)
83+
t.Run("IsValidJsonPortList",func(t*testing.T) {
84+
// 1. Check is a valid JSON
85+
// 2. Check is a valid port list (has address/protocol)
86+
stdout,_,err:=cli.Run("board","list","--json")
87+
require.NoError(t,err)
88+
requirejson.Parse(t,stdout).
89+
Query(`[ .detected_ports | .[].port | select(.address == null or .protocol == null) ]`).
90+
MustBeEmpty()
91+
})
10292

103-
// check is a valid json and contains a list of ports
104-
requirejson.Contains(t,stdout,`{
93+
t.Run("ContainsMockedPorts",func(t*testing.T) {
94+
stdout,_,err:=cli.Run("board","list","--json")
95+
require.NoError(t,err)
96+
requirejson.Contains(t,stdout,`{
10597
"detected_ports": [
10698
{
10799
"matching_boards": [
@@ -124,35 +116,20 @@ func TestBoardListMock(t *testing.T) {
124116
}
125117
}
126118
]
127-
}`)
128-
}
129-
130-
funcTestBoardListWithFqbnFilter(t*testing.T) {
131-
env,cli:=integrationtest.CreateArduinoCLIWithEnvironment(t)
132-
deferenv.CleanUp()
133-
134-
_,_,err:=cli.Run("core","update-index")
135-
require.NoError(t,err)
136-
137-
cli.InstallMockedSerialDiscovery(t)
138-
139-
stdout,_,err:=cli.Run("board","list","-b","foo:bar:baz","--json")
140-
require.NoError(t,err)
141-
requirejson.Query(t,stdout,`.detected_ports | length`,`0`)
142-
}
143-
144-
funcTestBoardListWithFqbnFilterInvalid(t*testing.T) {
145-
env,cli:=integrationtest.CreateArduinoCLIWithEnvironment(t)
146-
deferenv.CleanUp()
147-
148-
_,_,err:=cli.Run("core","update-index")
149-
require.NoError(t,err)
119+
}`)
120+
})
150121

151-
cli.InstallMockedSerialDiscovery(t)
122+
t.Run("ListWithFqbnFilter",func(t*testing.T) {
123+
stdout,_,err:=cli.Run("board","list","-b","foo:bar:baz","--json")
124+
require.NoError(t,err)
125+
requirejson.Query(t,stdout,`.detected_ports | length`,`0`)
126+
})
152127

153-
_,stderr,err:=cli.Run("board","list","-b","yadayada","--json")
154-
require.Error(t,err)
155-
requirejson.Query(t,stderr,".error",`"Invalid FQBN: not an FQBN: yadayada"`)
128+
t.Run("ListWithFqbnFilterInvalid",func(t*testing.T) {
129+
_,stderr,err:=cli.Run("board","list","-b","yadayada","--json")
130+
require.Error(t,err)
131+
requirejson.Query(t,stderr,".error",`"Invalid FQBN: not an FQBN: yadayada"`)
132+
})
156133
}
157134

158135
funcTestBoardListall(t*testing.T) {
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// This file is part of arduino-cli.
2+
//
3+
// Copyright 2025 ARDUINO SA (http://www.arduino.cc/)
4+
//
5+
// This software is released under the GNU General Public License version 3,
6+
// which covers the main part of arduino-cli.
7+
// The terms of this license can be found at:
8+
// https://www.gnu.org/licenses/gpl-3.0.en.html
9+
//
10+
// You can be released from the requirements of the above licenses by purchasing
11+
// a commercial license. Buying such a license is mandatory if you want to
12+
// modify or otherwise use the software for commercial activities involving the
13+
// Arduino software without disclosing the source code of your own applications.
14+
// To purchase a commercial license, send an email to license@arduino.cc.
15+
16+
package daemon
17+
18+
import (
19+
"fmt"
20+
"testing"
21+
"time"
22+
23+
"github.com/arduino/arduino-cli/internal/integrationtest"
24+
"github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
25+
"github.com/stretchr/testify/require"
26+
)
27+
28+
funcTestBoardListMock(t*testing.T) {
29+
env,cli:=integrationtest.CreateEnvForDaemon(t)
30+
deferenv.CleanUp()
31+
32+
_,_,err:=cli.Run("core","update-index")
33+
require.NoError(t,err)
34+
35+
cli.InstallMockedSerialDiscovery(t)
36+
37+
vartmp1,tmp2string
38+
39+
{
40+
// Create a new instance of the daemon
41+
grpcInst:=cli.Create()
42+
require.NoError(t,grpcInst.Init("","",func(ir*commands.InitResponse) {
43+
fmt.Printf("INIT> %v\n",ir.GetMessage())
44+
}))
45+
46+
// Run a BoardList
47+
resp,err:=grpcInst.BoardList(time.Second)
48+
require.NoError(t,err)
49+
require.NotEmpty(t,resp.Ports)
50+
for_,port:=rangeresp.Ports {
51+
ifport.GetPort().GetProtocol()=="serial" {
52+
tmp1=port.Port.GetProperties()["discovery_tmp"]
53+
}
54+
}
55+
require.NotEmpty(t,tmp1)
56+
57+
// Close instance
58+
require.NoError(t,grpcInst.Destroy(t.Context()))
59+
}
60+
61+
{
62+
// Create a second instance of the daemon
63+
grpcInst:=cli.Create()
64+
require.NoError(t,grpcInst.Init("","",func(ir*commands.InitResponse) {
65+
fmt.Printf("INIT> %v\n",ir.GetMessage())
66+
}))
67+
68+
// Run a BoardList
69+
resp,err:=grpcInst.BoardList(time.Second)
70+
require.NoError(t,err)
71+
require.NotEmpty(t,resp.Ports)
72+
for_,port:=rangeresp.Ports {
73+
ifport.GetPort().GetProtocol()=="serial" {
74+
tmp2=port.Port.GetProperties()["discovery_tmp"]
75+
}
76+
}
77+
require.NotEmpty(t,tmp2)
78+
79+
// Close instance
80+
require.NoError(t,grpcInst.Destroy(t.Context()))
81+
}
82+
83+
// Check if the discoveries have been successfully close
84+
require.NoFileExists(t,tmp1,"discovery has not been closed")
85+
require.NoFileExists(t,tmp2,"discovery has not been closed")
86+
}

‎internal/mock_serial_discovery/main.go‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,39 @@ package main
1919

2020
import (
2121
"errors"
22+
"fmt"
2223
"os"
2324
"time"
2425

26+
"github.com/arduino/go-paths-helper"
2527
"github.com/arduino/go-properties-orderedmap"
2628
discovery"github.com/arduino/pluggable-discovery-protocol-handler/v2"
2729
)
2830

2931
typemockSerialDiscoverystruct {
3032
startSyncCountint
3133
closeChanchan<-bool
34+
tmpFilestring
3235
}
3336

3437
funcmain() {
35-
dummy:=&mockSerialDiscovery{}
38+
// Write a file in a $TMP/mock_serial_discovery folder.
39+
// This file will be used by the integration tests to detect if the discovery is running.
40+
tmpDir:=paths.TempDir().Join("mock_serial_discovery")
41+
iferr:=tmpDir.MkdirAll();err!=nil {
42+
fmt.Fprintf(os.Stderr,"Error creating temp dir: %v\n",err)
43+
os.Exit(1)
44+
}
45+
tmpFile,err:=paths.MkTempFile(tmpDir,"")
46+
iferr!=nil {
47+
fmt.Fprintf(os.Stderr,"Error creating temp file: %v\n",err)
48+
os.Exit(1)
49+
}
50+
tmpFile.Close()
51+
deferos.Remove(tmpFile.Name())
52+
53+
// Run mock discovery
54+
dummy:=&mockSerialDiscovery{tmpFile:tmpFile.Name()}
3655
server:=discovery.NewServer(dummy)
3756
iferr:=server.Run(os.Stdin,os.Stdout);err!=nil {
3857
os.Exit(1)
@@ -80,9 +99,10 @@ func (d *mockSerialDiscovery) StartSync(eventCB discovery.EventCallback, errorCB
8099
ProtocolLabel:"Serial",
81100
HardwareID:"123456",
82101
Properties:properties.NewFromHashmap(map[string]string{
83-
"vid":"0x2341",
84-
"pid":"0x0041",
85-
"serial":"123456",
102+
"vid":"0x2341",
103+
"pid":"0x0041",
104+
"serial":"123456",
105+
"discovery_tmp":d.tmpFile,
86106
}),
87107
})
88108

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp