From 062047c640457bf8fb21e709cd3338dfd3c5ccfe Mon Sep 17 00:00:00 2001 From: Jason Pickens Date: Tue, 2 Jul 2019 21:59:34 +1200 Subject: [PATCH 1/2] Reproduce deadlock when parallize is false --- spec/integration/parallelize_spec.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/spec/integration/parallelize_spec.rb b/spec/integration/parallelize_spec.rb index 4cf4c52d..20a86be3 100644 --- a/spec/integration/parallelize_spec.rb +++ b/spec/integration/parallelize_spec.rb @@ -7,10 +7,16 @@ subject { shell(%w[git commit --allow-empty -m Test]) } let(:config) { <<-YML } + concurrency: 20 CommitMsg: TrailingPeriod: enabled: true parallelize: false + command: ['ruby', '-e', 'sleep 1'] + TextWidth: + enabled: true + parallelize: true + processors: 1 YML around do |example| @@ -22,6 +28,7 @@ end it 'does not hang' do - Timeout.timeout(5) { subject } + result = Timeout.timeout(5) { subject } + result.stderr.should_not include 'No live threads left. Deadlock?' end end From ce2ca4bbc6755175c486301ed06ef6b261ee29c0 Mon Sep 17 00:00:00 2001 From: Jason Pickens Date: Tue, 9 Jul 2019 09:53:07 +1200 Subject: [PATCH 2/2] Fix potential deadlock when waiting for hooks If a hook is running while another hook tries to start and there are not enough slots left then it will wait for the first hook to stop and release its slots. This first hook will not signal the second if there are no more hooks left. This results in a deadlock. This situation is far more likely to occur when parallelize is false since the hook will consume all the available slots. --- CHANGELOG.md | 1 + lib/overcommit/hook_runner.rb | 9 +++------ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78d6feb9..759a7434 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * Add `skipped_commit_types` option to `ReplaceBranch` prepare-commit-msg hook * Add `RubySyntax` pre-commit hook * Relax `childprocess` dependency to allow version 1.x +* Fix deadlock which was more likely to occur when setting `parallelize` on a hook to `false` ## 0.48.1 diff --git a/lib/overcommit/hook_runner.rb b/lib/overcommit/hook_runner.rb index 16af2c23..8ac01e99 100644 --- a/lib/overcommit/hook_runner.rb +++ b/lib/overcommit/hook_runner.rb @@ -129,12 +129,9 @@ def release_slot(hook) slots_released = processors_for_hook(hook) @slots_available += slots_released - if @hooks_left.any? - # Signal once. `wait_for_slot` will perform additional signals if - # there are still slots available. This prevents us from sending out - # useless signals - @resource.signal - end + # Signal every time in case there are threads that are already waiting for + # these slots to be released + @resource.signal end end