8000 Loadpoint: fix reentrant locks #2 by andig · Pull Request #18669 · evcc-io/evcc · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Loadpoint: fix reentrant locks #2 #18669

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Feb 8, 2025
Merged

Loadpoint: fix reentrant locks #2 #18669

merged 6 commits into from
Feb 8, 2025

Conversation

andig
Copy link
Member
@andig andig commented Feb 8, 2025 •
edited
Loading

Fix reentrant locks in #18603 (comment). Follow-up to #18650.

@andig andig added the bug Something isn't working label Feb 8, 2025
@andig andig requested a review from naltatis February 8, 2025 09:48
@andig andig force-pushed the fix/locks-2 branch 2 times, most recently from 25a22e7 to cd6a9f7 Compare February 8, 2025 09:54
@jeffborg
Copy link
Contributor
jeffborg commented Feb 8, 2025

@andig server doesn't startup even to port 7070 had to make some changes below is here is the pprof

1 @ 0x85de8 0x62418 0x623f5 0x8771c 0x9d3c4 0xd77eb8 0x8e604
#	0x8771b		sync.runtime_Semacquire+0x2b					runtime/sema.go:71
#	0x9d3c3		sync.(*WaitGroup).Wait+0x73					sync/waitgroup.go:118
#	0xd77eb7	github.com/eclipse/paho%2emqtt%2egolang.startComms.func3+0x27	github.com/eclipse/paho.mqtt.golang@v1.5.0/net.go:438

1 @ 0x85de8 0x62418 0x623f5 0x87868 0x1ed38d8 0x1ed3885 0x1ed3b4c 0x1ed3a58 0x1ec0e24 0x1ee107c 0x20cfbb0 0x5b76ec 0x5b7ec4 0x20ce014 0x20ce009 0x20e2a94 0x4de58 0x8e604
#	0x87867		sync.runtime_SemacquireRWMutexR+0x27					runtime/sema.go:100
#	0x1ed38d7	sync.(*RWMutex).RLock+0x77						sync/rwmutex.go:72
>>> #	0x1ed3884	github.com/evcc-io/evcc/core.(*Loadpoint).getMeasuredPhases+0x24	github.com/evcc-io/evcc/core/loadpoint_phases.go:45
#	0x1ed3b4b	github.com/evcc-io/evcc/core.(*Loadpoint).activePhases+0x3b		github.com/evcc-io/evcc/core/loadpoint_phases.go:73
>>> #	0x1ed3a57	github.com/evcc-io/evcc/core.(*Loadpoint).ActivePhases+0x67		github.com/evcc-io/evcc/core/loadpoint_phases.go:65
#	0x1ec0e23	github.com/evcc-io/evcc/core.(*Loadpoint).Prepare+0x963			github.com/evcc-io/evcc/core/loadpoint.go:664
#	0x1ee107b	github.com/evcc-io/evcc/core.(*Site).Prepare+0x15b			github.com/evcc-io/evcc/core/site.go:992
#	0x20cfbaf	github.com/evcc-io/evcc/cmd.runRoot+0x1b3f				github.com/evcc-io/evcc/cmd/root.go:307
#	0x5b76eb	github.com/spf13/cobra.(*Command).execute+0x81b				github.com/spf13/cobra@v1.8.1/command.go:989
#	0x5b7ec3	github.com/spf13/cobra.(*Command).ExecuteC+0x343			github.com/spf13/cobra@v1.8.1/command.go:1117
#	0x20ce013	github.com/spf13/cobra.(*Command).Execute+0x23				github.com/spf13/cobra@v1.8.1/command.go:1041
#	0x20ce008	github.com/evcc-io/evcc/cmd.Execute+0x18				github.com/evcc-io/evcc/cmd/root.go:118
#	0x20e2a93	main.main+0x43								github.com/evcc-io/evcc/main.go:53
#	0x4de57		runtime.main+0x287							runtime/proc.go:272

I removed lock in getMeasuredPhases just to get evcc to start, so far it seems ok. I'm writing values into most writeable values. So far seems ok.

If RLock/RUnlock can be mocked/intercepted in tests then would be easy to count and ensure via tests never called in a way to cause this issue in future.

@andig
Copy link
Member Author
andig commented Feb 8, 2025

@jeffborg great idea. Fixed bug and added test support. Should catch this type of error immediately without waiting for test timeout.

@andig andig merged commit f93ceff into master Feb 8, 2025
6 checks passed
@andig andig deleted the fix/locks-2 branch February 8, 2025 15:21
@andig andig mentioned this pull request Feb 8, 2025
Merged
andig added a commit that referenced this pull request Mar 15, 2025
@andig
Loadpoint: fix reentrant lock
8d3d42b 
Follow-up to #18669
andig added a commit that referenced this pull request Mar 15, 2025
@andig
Loadpoint: fix reentrant lock
3b30b27 
Follow-up to #18669
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@naltatis naltatis Awaiting requested review from naltatis

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@andig @jeffborg
0