Skip to content

Fix http.Transport copy-by-value causing race conditions#2999

Merged
turetske merged 2 commits intoPelicanPlatform:mainfrom
turetske:fix-transport-race-condition
Jan 16, 2026
Merged

Fix http.Transport copy-by-value causing race conditions#2999
turetske merged 2 commits intoPelicanPlatform:mainfrom
turetske:fix-transport-race-condition

Conversation

@turetske
Copy link
Collaborator

Replace Transport dereference pattern with Clone() to prevent concurrent map write panics. When http.Transport is copied by value (by dereferencing a pointer), it creates separate mutex copies that no longer protect the shared internal map, leading to race conditions.

Fixed instances:

  • oa4mp/proxy.go: Use Clone() instead of dereferencing *GetTransport() when customizing DialContext for Unix socket communication
  • web_ui/engine_test.go: Use Clone() for test transport customization

Also added go vet to the .pre-commit checker

Replace Transport dereference pattern with Clone() to prevent concurrent
map write panics. When http.Transport is copied by value (by dereferencing
a pointer), it creates separate mutex copies that no longer protect the
shared internal map, leading to race conditions.

Fixed instances:
- oa4mp/proxy.go: Use Clone() instead of dereferencing *GetTransport()
  when customizing DialContext for Unix socket communication
- web_ui/engine_test.go: Use Clone() for test transport customization

Also added `go vet` to the .pre-commit checker
@turetske turetske added bug Something isn't working critical High priority for next release create-patch labels Jan 16, 2026
@turetske turetske requested a review from h2zh January 16, 2026 19:31
@h2zh h2zh self-assigned this Jan 16, 2026
Copy link
Contributor

@h2zh h2zh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch for the transport object race condition. I also do a gerp for *config.GetTransport() to double check all occurrences are replaced. I got a concern that the newly added go-vet linter might be a duplication.

@turetske turetske requested a review from h2zh January 16, 2026 20:16
@h2zh h2zh assigned turetske and unassigned h2zh Jan 16, 2026
@turetske turetske assigned h2zh and unassigned h2zh and turetske Jan 16, 2026
@turetske turetske merged commit 49702e1 into PelicanPlatform:main Jan 16, 2026
26 of 28 checks passed
@h2zh
Copy link
Contributor

h2zh commented Jan 16, 2026

Sorry I didn't refresh the page (to get the new commit) before re-assigning this to you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working create-patch critical High priority for next release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants