websocket icon indicating copy to clipboard operation
websocket copied to clipboard

Published 20 hours ago • verified publisher icon nhooyr

fix closenow race

Open alixander opened this issue 1 year ago • 6 comments

the added test fails before this with

Screen Shot 2023-12-18 at 3 32 16 PM

i originally found this in D2 when i added tests for watching and --race started failing but the code seems to be purely confined in this library.

Screen Shot 2023-12-18 at 3 35 58 PM

hacked together test, so if you accept it and want me to clean it up, i can. just seeing if this is legit.

alixander avatar Dec 18 '23 23:12 alixander

Copy, weird. Will review tonight. Have a Christmas party now. Happy Holidays! 🥳

nhooyr avatar Dec 18 '23 23:12 nhooyr

Didn't get a chance, hopefully tomorrow.

nhooyr avatar Dec 19 '23 09:12 nhooyr

I despise the way this code was implemented. This is one of multiple races. See also #239

But this looks like the right fix thanks @alixander

I'll take a look at the test tonight. Helping cook Christmas meals at the Legion today.

nhooyr avatar Dec 19 '23 15:12 nhooyr

Merry Christmas man! Christmas in small towns sound 💯 .

I'm sure the test isn't up to par lol. Happy to address comments but also if you wanna just push to this or redo this in another PR, I'm cool with it.

alixander avatar Dec 19 '23 18:12 alixander

hey @alixander and @nhooyr , is it possible to merge this fix in? I am running into this issue on one of my projects.

thanks!

ClaytonNorthey92 avatar Feb 21 '24 19:02 ClaytonNorthey92

I know it's on my radar.

nhooyr avatar Feb 22 '24 03:02 nhooyr

Will have a release out by Sunday to fix this and all the associated bugs.

nhooyr avatar Apr 05 '24 23:04 nhooyr

Not sure why the Wasm test is failing, looking into it. Not sure if it's related to this PR or something else as it seems to fail intermittently in the daily CI.

nhooyr avatar Apr 06 '24 00:04 nhooyr