From 64a3218f5c7ba98a51c76065ed32ae860351e779 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Mon, 10 Jan 2022 23:24:58 -0700 Subject: 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? --- usagepool.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'usagepool.go') diff --git a/usagepool.go b/usagepool.go index 6fd48f5..96ed0f0 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() } -- cgit v1.2.3