[
https://issues.apache.org/jira/browse/YUNIKORN-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18112307#comment-18112307
]
Dale Richardson commented on YUNIKORN-3416:
-------------------------------------------
Correcting the two items this description left "with YUNIKORN-3370", since 3370
is being re-scoped and will not carry them.
* "{{{}stopped{}}} is never cleared": clean is actually an incorrect claim:.
{{GetUserGroupCache()}} has a done {{stopped.Store(false)}} inside the
{{once.Do}} that re-creates the instance since YUNIKORN-2233 (#752), so a cache
created after {{Stop()}} starts un-stopped.
* "{{{}GetUserGroupCache{}}}'s {{return instance}} still races the nil write":
turns out to be not verifiable. Two full {{pkg/shim}} runs under {{-race}} with
{{coreContext.StopAll()}} added to the shim's {{MockScheduler.stop()}} (so
every test now stops and re-creates the cache in-process) reported nothing. The
getter is a {{{}once{}}}-guarded read and the write is under {{{}c.lock{}}}, so
a report would need a concurrent {{Get}} during {{{}Stop(){}}}, which nothing
in either repo does.
Nothing further to do here; the receiver-locking fix that closed this stands.
> UserGroupCache cleanup locks the singleton instead of the receiver; nil
> dereference after Stop()
> ------------------------------------------------------------------------------------------------
>
> Key: YUNIKORN-3416
> URL: https://issues.apache.org/jira/browse/YUNIKORN-3416
> Project: Apache YuniKorn
> Issue Type: Sub-task
> Components: core - common
> Reporter: Dale Richardson
> Assignee: Wilfred Spiegelenburg
> Priority: Minor
> Fix For: 1.10.0
>
>
> {{cleanUpCache}} and {{resetCache}} lock the package-global {{instance}}
> while mutating the receiver's map {{c.ugs}}. In production the two are the
> same object, so the map is protected by accident. Where it goes wrong is
> {{Stop()}}: it sets {{instance = nil}}, and a cleaner run that has already
> picked its timer branch dereferences nil in {{cleanUpCache}}. That is a crash
> at shutdown (or on an in-process restart, YUNIKORN-3370). The non-singleton
> constructors are exported too, and such a cache mutates its map under a
> different cache's lock.
> Reproduced on the fork fix branch (the nil dereference and the non-singleton
> case both fail before the fix; the {{Stop()}} versus cleaner ordering shows
> under {{-race}}).
> Fix: lock the receiver ({{c.lock}}) in both functions. A fix exists on the
> tigerquoll fork ({{fix/usergroup-cache-receiver-locking}}, PR #18 there) and
> will be filed once this has a number. Not covered by it and left with
> YUNIKORN-3370: {{stopped}} is never cleared, and {{GetUserGroupCache}}'s
> {{return instance}} still races the nil write.
> Marker: three {{+checklocksignore}} sites in {{usergroup.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]
