Regression in async exception handling (?) on Windows in network-2.6.3.4

[snoyberg]

Sorry for the non-minimal repro, this is as far as I was able to take it for now. Consider the following Stack script, with uses LTS 10.7, which uses network-2.6.3.3.

#!/usr/bin/env stack
-- stack --resolver lts-10.7 script
{-# LANGUAGE OverloadedStrings #-}
import Network.Wai.Handler.Warp

main :: IO ()
main = testWithApplication (pure undefined) $ \_ -> pure ()

As expected, this exits immediately. However, if you change this to LTS-10.8 (which uses network-2.6.3.4), it will hang indefinitely on Windows. This regression has caused the Yesod test suite to fail, see yesodweb/yesod#1523.

I'm only guessing that this is a change in asynchronous exception behavior based on looking at the diff between 2.6.3.3 and 2.6.3.4, it could be something else at play.

[afcady]

This is the same as #326? I think the problem is that withMVar is used in accept now, where it used to be readMVar. Then the same MVar gets used in Network.Socket.close. I'm having some difficulty finding the source since everything was moved around. But you can see in efb0e79 it's currentStatus <- readMVar status and in the version on hackage it's withMVar status

Direct link to the line:

network/Network/Socket.hsc

Line 425 in efb0e79

currentStatus <- readMVar status

[afcady]

Here are the relevant lines:

network/Network/Socket.hsc

Lines 510 to 511 in cfa3f1f

	accept sock@(MkSocket s family stype protocol status) = do
	currentStatus <- readMVar status

network/Network/Socket.hsc

Lines 580 to 581 in aa49e91

	accept sock@(MkSocket s family stype protocol status) = withMVar status $ \currentStatus -> do

[eborden]

Yeah the change was originally in 0375259, which was to fix finalizers being called in GHC 8.2.2. However we abandoned the finalizer work because its behaviour was problematic (#302).

@afcady I would happily accept a PR to revert 0375259 and return to readMVar.

[kazu-yamamoto]

withMVar was necessary for mkWeakMVar in GHC 8.2.2.
But we have removed mkWeakMVar, so withMVar is not necessary anymore.
OK.
I will take care of this.

[kazu-yamamoto]

@snoyberg @eborden Should we fix in 2.7 only? Or both in 2.6 and 2.7?

[snoyberg]

Given that a lot of packages haven't upgraded to 2.7 yet, it would be great to have this backported to 2.6.

[kazu-yamamoto]

Just a question: Is it possible to release 2.6.x.y after 2.7.z.w is released in Hackage?

[snoyberg]

I don't see a problem with that.

…

On Mon, Jun 25, 2018, 9:14 AM Kazu Yamamoto ***@***.***> wrote: Just a question: Is it possible to release 2.6.x.y after 2.7.z.w is released in Hackage? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#327 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADBB07CbXLQRoxpLLfvpaTXG1ZBBM9iks5uAH-ogaJpZM4UtAnd> .

[eborden]

@kazu-yamamoto I'm going to put out the 2.6.x.x branch since many have not converted and this is a pretty sneaky insidious bug.

[kazu-yamamoto]

I'm going to put out the 2.6.x.x branch since many have not converted and this is a pretty sneaky insidious bug.

Thanks!

[eborden]

This has been fixed in 2.6.3.6.

[eborden]

Thanks @snoyberg and @afcady for hunting this one down!