Skip to content

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Sep 13, 2024

Previously, a pluggable discovery that crashed at startup would cause the client library to panic.

goroutine 11 [running]:
testing.tRunner.func1.2({0x5eaae0, 0x7db1f0})
	/home/cmaglie/Software/go1.22.3/src/testing/testing.go:1631 +0x24a
testing.tRunner.func1()
	/home/cmaglie/Software/go1.22.3/src/testing/testing.go:1634 +0x377
panic({0x5eaae0?, 0x7db1f0?})
	/home/cmaglie/Software/go1.22.3/src/runtime/panic.go:770 +0x132
github.com/arduino/pluggable-discovery-protocol-handler/v2.(*Client).Run(0xc0000fa090)
	/home/cmaglie/Workspace/pluggable-discovery-protocol-handler/client.go:281 +0x16e
github.com/arduino/pluggable-discovery-protocol-handler/v2.TestClient.func1(0xc0000b31e0)
	/home/cmaglie/Workspace/pluggable-discovery-protocol-handler/client_test.go:108 +0xcc
testing.tRunner(0xc0000b31e0, 0x64c850)
	/home/cmaglie/Software/go1.22.3/src/testing/testing.go:1689 +0xfb
created by testing.(*T).Run in goroutine 9
	/home/cmaglie/Software/go1.22.3/src/testing/testing.go:1742 +0x390
exit status 2

This PR fixes this problem and cleans up the mutex usage in the library.

Related to arduino/arduino-cli#2665.

Otherwise the call to:

   msg, err := disc.waitMessage(..)

may return both msg and err as nil, breaking the contract.
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 32.79%. Comparing base (c563e77) to head (cc38790).
Report is 4 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main      #50       +/-   ##
===========================================
+ Coverage   16.84%   32.79%   +15.95%     
===========================================
  Files           4        4               
  Lines         374      372        -2     
===========================================
+ Hits           63      122       +59     
+ Misses        294      228       -66     
- Partials       17       22        +5     
Flag Coverage Δ
unit 32.79% <100.00%> (+15.95%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cmaglie cmaglie added type: imperfection Perceived defect in any part of project topic: code Related to content of the project itself labels Sep 13, 2024
@cmaglie cmaglie merged commit fbec7bc into main Sep 18, 2024
32 checks passed
@cmaglie cmaglie deleted the fix_crashing_disc_handling branch September 18, 2024 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants