Skip to content

Comments

[fix] Fix possibly deadlock#680

Merged
ldmonster merged 6 commits intomainfrom
fix/possibly-deadlock-queue
Oct 7, 2025
Merged

[fix] Fix possibly deadlock#680
ldmonster merged 6 commits intomainfrom
fix/possibly-deadlock-queue

Conversation

@ldmonster
Copy link
Collaborator

@ldmonster ldmonster commented Oct 6, 2025

Overview

Replaced synchronization primitives with atomics to eliminate deadlock potential
Introduced a thread-safe status management system with TaskQueueStatus
Added deadlock prevention mechanisms in queue operations

What this PR does / why we need it

Special notes for your reviewer

Signed-off-by: Pavel Okhlopkov <pavel.okhlopkov@flant.com>
@ldmonster ldmonster self-assigned this Oct 6, 2025
@ldmonster ldmonster added the bug Something isn't working label Oct 6, 2025
Signed-off-by: Pavel Okhlopkov <pavel.okhlopkov@flant.com>
Signed-off-by: Pavel Okhlopkov <pavel.okhlopkov@flant.com>
Signed-off-by: Pavel Okhlopkov <pavel.okhlopkov@flant.com>
Signed-off-by: Pavel Okhlopkov <pavel.okhlopkov@flant.com>
@ldmonster ldmonster requested a review from Copilot October 7, 2025 07:43
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a potential deadlock issue by replacing queue iteration and filtering methods with their snapshot equivalents and updating function signatures to support context-based iteration.

  • Replace q.Iterate() with q.IterateSnapshot() to avoid holding locks during iteration
  • Replace q.Filter() with q.DeleteFunc() for safer task removal operations
  • Update dependency version and add context parameter to queue iteration methods

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/task/service/converge.go Replace Iterate with IterateSnapshot for counting tasks
pkg/task/queue/queue.go Replace Iterate with IterateSnapshot and Filter with DeleteFunc
pkg/module_manager/module_manager.go Replace Iterate with IterateSnapshot for task enumeration
pkg/addon-operator/queue_test.go Update test code to use IterateSnapshot
pkg/addon-operator/queue.go Replace Iterate with IterateSnapshot and Filter with DeleteFunc
pkg/addon-operator/operator_test.go Add prometheus registry setup and use IterateSnapshot
pkg/addon-operator/metrics.go Add context parameter to queue iteration method
go.mod Update shell-operator dependency and prometheus client

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Signed-off-by: Pavel Okhlopkov <pavel.okhlopkov@flant.com>
@ldmonster ldmonster merged commit e1aafa2 into main Oct 7, 2025
8 of 9 checks passed
@ldmonster ldmonster deleted the fix/possibly-deadlock-queue branch October 7, 2025 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant