Skip to content

Prevent gin.SetMode from causing race conditions#2330

Merged
djeebus merged 4 commits intomainfrom
fix-gin-race-condition
Apr 8, 2026
Merged

Prevent gin.SetMode from causing race conditions#2330
djeebus merged 4 commits intomainfrom
fix-gin-race-condition

Conversation

@djeebus
Copy link
Copy Markdown
Contributor

@djeebus djeebus commented Apr 8, 2026

  1. We run all of our tests in parallel.
  2. We fail our tests if data race conditions are detected.
  3. Many tests call gin.SetMode
  4. gin.SetMode writes a global variable
  5. Therefore, calling gin.SetMode in multiple tests that run in parallel can cause data race conditions.

This PR removes them and uses the environment variable to gain the same effect. We set GIN_MODE=test when running tests in github actions, and GIN_MODE=release when deploying any jobs.

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 8, 2026

PR Summary

Medium Risk
Shifts Gin mode configuration from in-process gin.SetMode calls to environment variables, which can change runtime behavior if GIN_MODE is not set correctly in deploy/test environments. CI and Nomad defaults mitigate this, but missing env wiring could lead to unexpected logging/debug behavior.

Overview
Removes all in-code gin.SetMode(...) usage (including test init/TestMain and service startup paths) to avoid race conditions from Gin’s global mode, and instead standardizes mode selection via GIN_MODE (set to test in PR GitHub Actions workflows and defaulted to release for the API Nomad job). Also adds a golangci-lint forbidigo rule to prevent reintroducing gin.SetMode.

Reviewed by Cursor Bugbot for commit 9805594. Bugbot is set up for automated code reviews on this repo. Configure here.

Copy link
Copy Markdown
Contributor

@levb levb left a comment

Choose a reason for hiding this comment

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

LGTM, tests pending

Copy link
Copy Markdown
Member

@jakubno jakubno left a comment

Choose a reason for hiding this comment

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

can we just set it to release mode when starting gin server in code? I don't think there's any benefit for us to use debug ever

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2a5023b. Configure here.

already set via env var in the nomad job
@djeebus djeebus merged commit 473fd63 into main Apr 8, 2026
44 checks passed
@djeebus djeebus deleted the fix-gin-race-condition branch April 8, 2026 20:35
@djeebus
Copy link
Copy Markdown
Contributor Author

djeebus commented Apr 9, 2026

can we just set it to release mode when starting gin server in code? I don't think there's any benefit for us to use debug ever

There is lots of very useful output that gin provides with mode=debug that can help with debugging local issues. It makes perfect sense to run it locally with debug, but disable it when deploying. It can also be useful to turn it on temporarily when deployed, to identify mismatches in paths, routes, requests sent vs requests received, things like that. Seems silly to prevent ourselves from using it when turning it off only when deploying is so simple.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants