[
https://issues.apache.org/jira/browse/YUNIKORN-3405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18108176#comment-18108176
]
Manikandan R commented on YUNIKORN-3405:
----------------------------------------
[~tigerquoll] has given comments on
[https://github.com/apache/yunikorn-k8shim/pull/1074
|https://github.com/apache/yunikorn-k8shim/pull/1074]related to this Jira and
the same has to be addressed.
> Dispatcher Start and Stop operations have race conditions
> ---------------------------------------------------------
>
> Key: YUNIKORN-3405
> URL: https://issues.apache.org/jira/browse/YUNIKORN-3405
> Project: Apache YuniKorn
> Issue Type: Bug
> Components: shim - kubernetes
> Reporter: Andy Huang
> Priority: Major
>
> The Kubernetes shim dispatcher lifecycle implementation uses separate checks
> and state updates that are not atomic across the complete Start() and Stop()
> operations.
> Dispatcher.Start() currently checks isRunning(), replaces stopChan, updates a
> WaitGroup, starts a goroutine, and only then marks the dispatcher as running.
> Concurrent Start() calls can both observe running=false and proceed. This can
> result in:
> * multiple dispatcher goroutines being started
> * stopChan being replaced while another goroutine still references it
> * a dispatcher goroutine becoming unreachable by Stop()
> * inconsistent WaitGroup accounting
> * leaked goroutines
> Dispatcher.Stop() first checks whether stopChan is closed and later closes it
> as a separate operation. Concurrent Stop() calls can both observe an open
> channel and attempt to close it, potentially causing a
> close-of-closed-channel panic.
> Start() and Stop() may also race with each other while stopChan and the
> running state are being updated independently.
> The dispatcher lifecycle should be made thread-safe, idempotent, and
> explicit. All state transitions involving running, stopChan, and the shutdown
> WaitGroup should be coordinated as one operation.
> This was identified while reviewing the KubernetesShim shutdown fixes in:
> * YUNIKORN-3367
> * https://github.com/apache/yunikorn-k8shim/pull/1074
> Relevant code:
> * pkg/dispatcher/dispatcher.go
> * dispatcher.Start()
> * dispatcher.Stop()
> Expected behavior
> * Only one dispatcher event-processing goroutine can run.
> * Concurrent Start() calls do not create duplicate or unreachable goroutines.
> * Stop() is safe and idempotent.
> * Concurrent Stop() calls do not panic.
> * Start() and Stop() cannot leave stopChan, the running state, or WaitGroup
> accounting inconsistent.
> * Stop() waits for the active dispatcher goroutine to terminate, subject to
> the existing shutdown timeout.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
