Andy Huang created YUNIKORN-3404:
------------------------------------
Summary: PlaceholderManager Start and Stop operations have race
conditions
Key: YUNIKORN-3404
URL: https://issues.apache.org/jira/browse/YUNIKORN-3404
Project: Apache YuniKorn
Issue Type: Bug
Components: shim - kubernetes
Reporter: Andy Huang
The Kubernetes shim PlaceholderManager lifecycle implementation has several
race conditions around its running state and stop channel.
PlaceholderManager.Start() currently checks the running atomic and updates it
in two separate operations:
{code:go}
if mgr.isRunning() {
return
}
mgr.setRunning(true)
go func() {
// ...
}()
{code}
Concurrent Start() calls can both observe running=false before either call
stores true. This may start multiple cleanup goroutines for the same
PlaceholderManager.
PlaceholderManager.Stop() has a similar check-then-act pattern:
{code:go}
if !mgr.isRunning() {
return
}
mgr.stopChan <- struct{}{}
{code}
Concurrent Stop() calls can both observe running=true. Only one send can be
received before the cleanup goroutine exits, potentially leaving another caller
blocked indefinitely on the unbuffered stop channel.
The running state is changed asynchronously by the cleanup goroutine after
receiving the stop signal. Stop() therefore does not guarantee that the manager
is fully stopped when it returns, and lifecycle state can become inconsistent
when Start() and Stop() are invoked concurrently.
The lifecycle operations should be made atomic and idempotent. Possible
approaches include CompareAndSwap for state transitions, closing a stop channel
exactly once, and waiting for the cleanup goroutine to exit.
This was identified while reviewing the KubernetesShim shutdown fixes in:
* YUNIKORN-3367
* YUNIKORN-3368
* https://github.com/apache/yunikorn-k8shim/pull/1074
Relevant code:
* pkg/cache/placeholder_manager.go
* PlaceholderManager.Start()
* PlaceholderManager.Stop()
{code}
**Expected behavior**
```text
* At most one PlaceholderManager cleanup goroutine can run.
* Concurrent Start() calls do not start duplicate goroutines.
* Stop() is safe and idempotent.
* Concurrent Stop() calls do not block or panic.
* When Stop() returns, the manager has either fully stopped or exposes a
reliable way to wait for shutdown completion.
* Start() and Stop() do not leave the running state inconsistent.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
