Skip to content

Commit 42d7fee

Browse files
authored
Merge pull request kubernetes#80788 from tedyu/stop-thread-unsafe
Rename cacheWatcher#stop
2 parents f6bc0ea + 0b3c07a commit 42d7fee

File tree

2 files changed

+10
-13
lines changed

2 files changed

+10
-13
lines changed

staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -862,7 +862,8 @@ func (c *Cacher) startDispatchingBookmarkEvents() {
862862
// as we don't delete watcher from bookmarkWatchers when it is stopped.
863863
for _, watchers := range c.bookmarkWatchers.popExpiredWatchers() {
864864
for _, watcher := range watchers {
865-
// watcher.stop() is protected by c.Lock()
865+
// c.Lock() is held here.
866+
// watcher.stopThreadUnsafe() is protected by c.Lock()
866867
if watcher.stopped {
867868
continue
868869
}
@@ -932,7 +933,7 @@ func (c *Cacher) finishDispatching() {
932933
defer c.Unlock()
933934
c.dispatching = false
934935
for _, watcher := range c.watchersToStop {
935-
watcher.stop()
936+
watcher.stopThreadUnsafe()
936937
}
937938
c.watchersToStop = c.watchersToStop[:0]
938939
}
@@ -947,7 +948,7 @@ func (c *Cacher) stopWatcherThreadUnsafe(watcher *cacheWatcher) {
947948
if c.dispatching {
948949
c.watchersToStop = append(c.watchersToStop, watcher)
949950
} else {
950-
watcher.stop()
951+
watcher.stopThreadUnsafe()
951952
}
952953
}
953954

@@ -977,7 +978,7 @@ func forgetWatcher(c *Cacher, index int, triggerValue string, triggerSupported b
977978
defer c.Unlock()
978979

979980
// It's possible that the watcher is already not in the structure (e.g. in case of
980-
// simultaneous Stop() and terminateAllWatchers(), but it is safe to call stop()
981+
// simultaneous Stop() and terminateAllWatchers(), but it is safe to call stopThreadUnsafe()
981982
// on a watcher multiple times.
982983
c.watchers.deleteWatcher(index, triggerValue, triggerSupported, c.stopWatcherThreadUnsafe)
983984
}
@@ -1079,8 +1080,8 @@ func (c *errWatcher) Stop() {
10791080
}
10801081

10811082
// cacheWatcher implements watch.Interface
1083+
// this is not thread-safe
10821084
type cacheWatcher struct {
1083-
sync.Mutex
10841085
input chan *watchCacheEvent
10851086
result chan watch.Event
10861087
done chan struct{}
@@ -1121,12 +1122,8 @@ func (c *cacheWatcher) Stop() {
11211122
c.forget()
11221123
}
11231124

1124-
// TODO(#73958)
1125-
// stop() is protected by Cacher.Lock(), rename it to
1126-
// stopThreadUnsafe and remove the sync.Mutex.
1127-
func (c *cacheWatcher) stop() {
1128-
c.Lock()
1129-
defer c.Unlock()
1125+
// we rely on the fact that stopThredUnsafe is actually protected by Cacher.Lock()
1126+
func (c *cacheWatcher) stopThreadUnsafe() {
11301127
if !c.stopped {
11311128
c.stopped = true
11321129
close(c.done)

staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func TestCacheWatcherCleanupNotBlockedByResult(t *testing.T) {
6363
// forget() has to stop the watcher, as only stopping the watcher
6464
// triggers stopping the process() goroutine which we are in the
6565
// end waiting for in this test.
66-
w.stop()
66+
w.stopThreadUnsafe()
6767
}
6868
initEvents := []*watchCacheEvent{
6969
{Object: &v1.Pod{}},
@@ -472,7 +472,7 @@ func TestCacheWatcherStoppedInAnotherGoroutine(t *testing.T) {
472472
done := make(chan struct{})
473473
filter := func(string, labels.Set, fields.Set) bool { return true }
474474
forget := func() {
475-
w.stop()
475+
w.stopThreadUnsafe()
476476
done <- struct{}{}
477477
}
478478

0 commit comments

Comments
 (0)