arduino/arduino-create-agentPublic

NotificationsYou must be signed in to change notification settings
Fork150
Star441

fix: data race on`Systray.SetCurrentConfigFile`#1012

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Open

dido18 wants to merge18 commits intomain

base:main

Choose a base branch

fromdatarace-on-config-access

Open

fix: data race on`Systray.SetCurrentConfigFile`#1012

dido18 wants to merge18 commits intomainfromdatarace-on-config-access

Conversation

Copy link

Contributor

dido18 commentedJan 21, 2025•
edited
Loading

Please check if the PR fulfills these requirements

The PR has no duplicates (please search among thePull Requests
before creating one)
Tests for the changes have been added (for bug fixes / features)

What kind of change does this PR introduce?
Fix a data race on the config set to the Systray struct.

What is the current behavior?
When the agent starts, there is a data race on theSetCurrentConfigFile method called by two different go routines.

2025/01/10 14:35:07 stderr: WARNING: DATA RACE2025/01/10 14:35:07 stderr: Write at 0x0000018b31e8 by goroutine 16:2025/01/10 14:35:07 stderr: github.com/arduino/arduino-create-agent/systray.(*Systray).SetCurrentConfigFile()2025/01/10 14:35:07 stderr: /home/dido/code/arduino/arduino-create-agent/systray/systray.go:102 +0x79d2025/01/10 14:35:07 stderr: main.loop()2025/01/10 14:35:07 stderr: /home/dido/code/arduino/arduino-create-agent/main.go:261 +0x7912025/01/10 14:35:07 stderr:2025/01/10 14:35:07 stderr: Previous write at 0x0000018b31e8 by main goroutine:2025/01/10 14:35:07 stderr: main.main()2025/01/10 14:35:07 stderr: /home/dido/code/arduino/arduino-create-agent/main.go:149 +0x2392025/01/10 14:35:07 stderr:2025/01/10 14:35:07 stderr: Goroutine 16 (running) created at:2025/01/10 14:35:07 stderr: main.main()2025/01/10 14:35:07 stderr: /home/dido/code/arduino/arduino-create-agent/main.go:145 +0xfa2025/01/10 14:35:07 stderr: ==================

What is the new behavior?
The config is read in the main and theSetCurrentConfigFile on Systray is called by the main goroutine (and not in the otherloop go routine)

Does this PR introduce a breaking change?

Other information:

Move the logic to read the config.ini file into the '/config/config.go` module
Add Unit tests (only for LINUX OS)
Testing thelegacy behaviours is not easy (the config.ini file in the old location is copied into the new home location). It relies on theos.Executable() calls

fix

90cce2c

dido18 mentioned this pull request

Jan 21, 2025

fix: datarace config read/write by multiple go routines#1011

Closed

2 tasks

dido18 added4 commits

January 21, 2025 11:33

feat(config): add default configuration file and update config handling

c96490b

fix(.gitignore): add entry to ignore config.ini file

7b4859a

test(config): add test for writing default config.ini file and update…

279de2a

… test data

test(config): add tests for retrieving config paths from XDG_CONFIG_H…

4f3ac5a

…OME and HOME directories

Copy link

codecov-commenter commentedJan 21, 2025•
edited
Loading

Codecov Report

Attention: Patch coverage is58.97436% with16 lines in your changes missing coverage. Please review.

Project coverage is 21.59%. Comparing base(1b94ccc) to head(870fac5).

Files with missing lines	Patch %	Lines
config/config.go	74.19%	7 Missing and 1 partial⚠️
main.go	0.00%	8 Missing⚠️

Additional details and impacted files

@@            Coverage Diff             @@##             main    #1012      +/-   ##==========================================+ Coverage   20.14%   21.59%   +1.44%==========================================  Files          42       42                Lines        3222     3228       +6     ==========================================+ Hits          649      697      +48+ Misses       2488     2442      -46- Partials       85       89       +4