aboutsummaryrefslogtreecommitdiffhomepage
path: root/usagepool.go
diff options
context:
space:
mode:
authorMatthew Holt <[email protected]>2022-01-10 23:24:58 -0700
committerMatthew Holt <[email protected]>2022-01-10 23:24:58 -0700
commit64a3218f5c7ba98a51c76065ed32ae860351e779 (patch)
tree7c73bf805d6f0cf483e86e6bcba94bd8527b44e5 /usagepool.go
parentc634bbe9cc7ef6ce6f9f776010ce96384fd43340 (diff)
downloadcaddy-64a3218f5c7ba98a51c76065ed32ae860351e779.tar.gz
caddy-64a3218f5c7ba98a51c76065ed32ae860351e779.zip
core: Simplify shared listeners, fix deadline bug
When this listener code was first written, UsagePool didn't exist. We can simplify much of the wrapped listener logic by utilizing UsagePool. This also fixes a bug where new servers were able to clear deadlines set by old servers, even if the old server didn't get booted out of its Accept() call yet. And with the deadline cleared, they never would. (Sometimes. Based on reports and difficulty of reproducing the bug, this behavior was extremely rare.) I don't know why that happened exactly, maybe some polling mechanism in the kernel and if the timings worked out just wrong it would expose the bug. Anyway, now we ensure that only the closer that set the deadline is the same one that clears it, ensuring that old servers always return out of Accept(), because the deadline doesn't get cleared until they do. Of course, all this hinges on the hope that my suspicions in the middle of the night are correct and that kernels work the way I think they do in my head. Also minor enhancement to UsagePool where if a value errors upon construction (a very real possibility with listeners), it is removed from the pool. Not 100% sure the sync logic is correct there, or maybe we don't have to even put it in the pool until after construction, but it's subtle either way and I think this is safe... right?
Diffstat (limited to 'usagepool.go')
-rw-r--r--usagepool.go9
1 files changed, 8 insertions, 1 deletions
diff --git a/usagepool.go b/usagepool.go
index 6fd48f5b4..96ed0f0ef 100644
--- a/usagepool.go
+++ b/usagepool.go
@@ -94,8 +94,15 @@ func (up *UsagePool) LoadOrNew(key interface{}, construct Constructor) (value in
if err == nil {
upv.value = value
} else {
- // TODO: remove error'ed entries from map
upv.err = err
+ up.Lock()
+ // this *should* be safe, I think, because we have a
+ // write lock on upv, but we might also need to ensure
+ // that upv.err is nil before doing this, since we
+ // released the write lock on up during construct...
+ // but then again it's also after midnight...
+ delete(up.pool, key)
+ up.Unlock()
}
upv.Unlock()
}