abolish an extra goroutine #501
Conversation
|
sorry for all the commit spam, i've read a tweet sometime ago that github actions is a fiend to clean git commit history, while i think it's true it's my fault i'm too eepy and couldn't be bothered to run the workflow locally, check up .mod files and not to forgor about running go mod tidy. think this is like your average dark souls bonfire-to-"you died" message cycle but somewhat upside down, fix issue -> commit -> push -> wait for ci to fail -> oh my gosh here we go again. please review my pr carefully, i am super new to the codebase although i've been using this library for a while and i might've messed up choosing atomic.Value vs atomic.Pointer but the latter looks more messy to me code wise and i don't think there is any sense to keep a pointer to function instead of keeping it straight as a value, maybe someone smarter than me and/or more acquainted with the codebase than me could propose anything. i tried my best to do the best choices and keep the style consistent. maybe you'll find an nit or two but i'm up for fixing them up. |
| writeTimeout chan context.Context | ||
| timeoutLoopDone chan struct{} | ||
| readTimeoutCloser atomic.Value | ||
| writeTimeoutCloser atomic.Value |
There was a problem hiding this comment.
I think atomic.Pointer would be cleaner. atomic.Value still stores the value as a pointer, it's just not explicit. The upside of atomic.Pointer is that it requires no type casting.
| c.close() | ||
| return | ||
| func (c *Conn) clearWriteTimeout() { | ||
| if closer := c.writeTimeoutCloser.Load(); closer != nil { |
There was a problem hiding this comment.
Shouldn't this be a swap too to remove the reference to the function to let GC take it? Same below.
| defer close(c.timeoutLoopDone) | ||
| func (c *Conn) setupWriteTimeout(ctx context.Context) { | ||
| hammerTime := context.AfterFunc(ctx, func() { | ||
| c.close() |
There was a problem hiding this comment.
I wonder if it'd be possible for this function to also remove itself from c.writeTimeoutCloser? Perhaps it can put a nil in it / or swap it with a nil and run what was in it if it's not nil. That way we'll let GC collect the function once its ran. Same below.
Well, it just needs to call clearWriteTimeout().
closes #411