[
https://issues.apache.org/jira/browse/YUNIKORN-3419?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Dale Richardson updated YUNIKORN-3419:
--------------------------------------
Description:
Seven sites in {{queue.go}} read a lock-guarded field without the lock. None is
a crash; they are torn reads that feed a message, a decision or an event.
Grouped because the fix is the same everywhere: read through the locked
accessor into a local first.
* {{TryIncAllocatedResource}} and {{DecAllocatedResource}} read {{maxResource}}
and {{allocatedResource}} for their error text. The lock is deliberately not
held (parent-first recursion), so the values go into the message unlocked.
* {{findPreemptionFenceRoot}} reads {{allocatedResource}} while walking up the
hierarchy, same constraint.
* {{MarkQueueForRemoval}} re-reads {{sq.children}} after {{doRemoveQueue()}}
released the lock, although it already holds a copy that the loop walks; test
the copy.
* {{getOutStandingRequestsInternal}} reads {{app.queuePath}} from the loop
variable without the application lock ({{app.user}} on the same line is
immutable after construction and is fine); use {{GetQueuePath()}}.
* {{NewConfiguredQueue}} and {{newDynamicQueueInternal}} read {{isManaged}} for
the new-queue event after {{parent.addChildQueue}} has published the queue, so
a config reload that flips {{isManaged}} on that queue ({{ApplyConf}}) can race
the read; read it into a local before publishing.
Marker: seven {{+checklocksignore}} sites in {{queue.go}} carry this JIRA; the
fix removes them.
was:
Seven sites in {{queue.go}} read a lock-guarded field without the lock. None is
a crash; they are torn reads that feed a message, a decision or an event.
Grouped because the fix is the same everywhere: read through the locked
accessor into a local first.
* {{TryIncAllocatedResource}} and {{DecAllocatedResource}} read {{maxResource}}
and {{allocatedResource}} for their error text. The lock is deliberately not
held (parent-first recursion), so the values go into the message unlocked.
* {{findPreemptionFenceRoot}} reads {{allocatedResource}} while walking up the
hierarchy, same constraint.
* {{MarkQueueForRemoval}} re-reads {{sq.children}} after {{doRemoveQueue()}}
released the lock, although it already holds a copy that the loop walks; test
the copy.
* {{getOutStandingRequestsInternal}} reads {{app.queuePath}} and {{app.user}}
from the loop variable without the application lock; use {{GetQueuePath()}} and
a locked user accessor.
* {{NewConfiguredQueue}} and {{newDynamicQueueInternal}} read {{isManaged}} for
the new-queue event after {{parent.addChildQueue}} has published the queue, so
a config reload that flips {{isManaged}} on that queue ({{ApplyConf}}) can race
the read; read it into a local before publishing.
Marker: seven {{+checklocksignore}} sites in {{queue.go}} carry this JIRA; the
fix removes them.
> Queue fields read without the queue lock in parent-first paths and the
> constructors
> -----------------------------------------------------------------------------------
>
> Key: YUNIKORN-3419
> URL: https://issues.apache.org/jira/browse/YUNIKORN-3419
> Project: Apache YuniKorn
> Issue Type: Sub-task
> Components: core - scheduler
> Reporter: Dale Richardson
> Assignee: jimmycmlin
> Priority: Minor
>
> Seven sites in {{queue.go}} read a lock-guarded field without the lock. None
> is a crash; they are torn reads that feed a message, a decision or an event.
> Grouped because the fix is the same everywhere: read through the locked
> accessor into a local first.
> * {{TryIncAllocatedResource}} and {{DecAllocatedResource}} read
> {{maxResource}} and {{allocatedResource}} for their error text. The lock is
> deliberately not held (parent-first recursion), so the values go into the
> message unlocked.
> * {{findPreemptionFenceRoot}} reads {{allocatedResource}} while walking up
> the hierarchy, same constraint.
> * {{MarkQueueForRemoval}} re-reads {{sq.children}} after {{doRemoveQueue()}}
> released the lock, although it already holds a copy that the loop walks; test
> the copy.
> * {{getOutStandingRequestsInternal}} reads {{app.queuePath}} from the loop
> variable without the application lock ({{app.user}} on the same line is
> immutable after construction and is fine); use {{GetQueuePath()}}.
> * {{NewConfiguredQueue}} and {{newDynamicQueueInternal}} read {{isManaged}}
> for the new-queue event after {{parent.addChildQueue}} has published the
> queue, so a config reload that flips {{isManaged}} on that queue
> ({{ApplyConf}}) can race the read; read it into a local before publishing.
> Marker: seven {{+checklocksignore}} sites in {{queue.go}} carry this JIRA;
> the fix removes them.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
