Fix phase check race condition in Puma::Cluster#check_workers#3690
Fix phase check race condition in Puma::Cluster#check_workers#3690
Puma::Cluster#check_workers#3690Conversation
| def test_application_is_loaded_exactly_once_if_using_fork_worker | ||
| cli_server "-w #{workers} --fork-worker test/rackup/write_to_stdout_on_boot.ru" | ||
|
|
||
| get_worker_pids | ||
| loading_app_count = @server_log.scan("Loading app").length | ||
| assert_equal 1, loading_app_count # loaded in worker 0 | ||
|
|
||
| @server_log.clear | ||
| Process.kill :SIGURG, @pid | ||
|
|
||
| get_worker_pids 1, workers - 1 | ||
| loading_app_count = @server_log.scan("Loading app").length | ||
| assert_equal 0, loading_app_count | ||
| end |
There was a problem hiding this comment.
Here's an intermittent test failure when running this consecutively locally without the patch:
puma [fix-check-workers-phase-race-condition] bundle exec m test/test_integration_cluster.rb:495
Run options: -n "/^(test_application_is_loaded_exactly_once_if_using_fork_worker)$/" --seed 33914
# Running:
.
Fabulous run in 5.410711s, 0.1848 runs/s, 0.5545 assertions/s.
1 runs, 3 assertions, 0 failures, 0 errors, 0 skips
puma [fix-check-workers-phase-race-condition] bundle exec m test/test_integration_cluster.rb:495
Run options: -n "/^(test_application_is_loaded_exactly_once_if_using_fork_worker)$/" --seed 50616
# Running:
.
Fabulous run in 5.389803s, 0.1855 runs/s, 0.5566 assertions/s.
1 runs, 3 assertions, 0 failures, 0 errors, 0 skips
puma [fix-check-workers-phase-race-condition] bundle exec m test/test_integration_cluster.rb:495
Run options: -n "/^(test_application_is_loaded_exactly_once_if_using_fork_worker)$/" --seed 58475
# Running:
F
Fabulous run in 5.397754s, 0.1853 runs/s, 0.5558 assertions/s.
Errors & Failures:
1) Failure:
TestIntegrationCluster#test_application_is_loaded_exactly_once_if_using_fork_worker [test/test_integration_cluster.rb:507]:
Expected: 0
Actual: 1
1 runs, 3 assertions, 1 failures, 0 errors, 0 skips
There was a problem hiding this comment.
And here's one from a CI build: https://github.com/puma/puma/actions/runs/16876344124/job/47801719812?pr=3690
Whereas the other builds are green: https://github.com/puma/puma/actions/runs/16876344124
There was a problem hiding this comment.
I've pushed the patch, we shouldn't see any failures now: https://github.com/puma/puma/actions/runs/16876978063
280038a to
2e06134
Compare
|
I just swapped the default branch name from When you rebase in the future, use |
2e06134 to
0211c91
Compare
|
Merging as I've been able to validate the fix locally and via CI, both here and in the PR this was extracted from. |
Extracts some code from #3602. Namely: