natefinch/npipe/v2 does not support the POSIX read/close semantics juju needs

[David Cheney]

TL;DR read issue https://bugs.launchpad.net/bugs/1581337

Thumper asked me to forward this to juju-dev for discussion

http://reports.vapour.ws/releases/3964/job/run-unit-tests-win2012-amd64/attempt/2384

Fails because, npipe.Accept is blocking on accept

goroutine 41 [syscall, locked to thread]:
syscall.Syscall(0x7f8139f102c, 0x2, 0x1a8, 0xffffffff, 0x0,
0xc082183de8, 0x3e5, 0x1c0050)
 C:/Go/src/runtime/syscall_windows.go:163 +0x5c
syscall.WaitForSingleObject(0x1a8, 0xc0ffffffff, 0xc082183e10, 0x0, 0x0)
 C:/Go/src/syscall/zsyscall_windows.go:693 +0x6f
gopkg.in/natefinch/npipe%2ev2.waitForCompletion(0x15c, 0xc0822a42a0,
0xc08202b880, 0x0, 0x0)
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/gopkg.in/natefinch/npipe.v2/npipe_windows.go:181
+0x45
gopkg.in/natefinch/npipe%2ev2.(*PipeListener).AcceptPipe(0xc08202b880,
0x0, 0x0, 0x0)
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/gopkg.in/natefinch/npipe.v2/npipe_windows.go:307
+0x397
gopkg.in/natefinch/npipe%2ev2.(*PipeListener).Accept(0xc08202b880,
0x0, 0x0, 0x0, 0x0)
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/gopkg.in/natefinch/npipe.v2/npipe_windows.go:261
+0x44
github.com/juju/juju/worker/metrics/spool.(*socketListener).loop(0xc08227e780,
0x0, 0x0)
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/github.com/juju/juju/worker/metrics/spool/listener.go:71
+0xaa
github.com/juju/juju/worker/metrics/spool.NewSocketListener.func1(0xc08227e780)
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/github.com/juju/juju/worker/metrics/spool/listener.go:43
+0x65
created by github.com/juju/juju/worker/metrics/spool.NewSocketListener
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/github.com/juju/juju/worker/metrics/spool/listener.go:44
+0x140

and

socketListener is waiting on the tomb to hit done.

goroutine 43 [chan receive]:
launchpad.net/tomb.(*Tomb).Wait(0xc08227e790, 0x0, 0x0)
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/launchpad.net/tomb/tomb.go:108
+0x5f
github.com/juju/juju/worker/metrics/spool.(*socketListener).Stop(0xc08227e780)
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/github.com/juju/juju/worker/metrics/spool/listener.go:56
+0x18c
github.com/juju/juju/worker/metrics/sender.(*sender).stop(0xc082269770)
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/github.com/juju/juju/worker/metrics/sender/sender.go:104
+0x45
github.com/juju/juju/worker/metrics/sender.(*sender).(github.com/juju/juju/worker/metrics/sender.stop)-fm()
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/github.com/juju/juju/worker/metrics/sender/manifold.go:77
+0x27
github.com/juju/juju/worker/metrics/spool.(*periodicWorker).Kill(0xc0822a40e0)
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/github.com/juju/juju/worker/metrics/spool/listener.go:103
+0x28
github.com/juju/juju/worker/metrics/sender_test.(*ManifoldSuite).setupWorkerTest.func1(0xc0822791d0)
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/github.com/juju/juju/worker/metrics/sender/manifold_test.go:101
+0x3d
github.com/juju/testing.(*CleanupSuite).callStack(0xc0821a9a28,
0xc0822791d0, 0xc08226bdd0, 0x2, 0x2)
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/github.com/juju/testing/cleanup.go:52
+0x51
github.com/juju/testing.(*CleanupSuite).TearDownTest(0xc0821a9a28, 0xc0822791d0)
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/github.com/juju/testing/cleanup.go:46
+0x4d
github.com/juju/testing.(*IsolationSuite).TearDownTest(0xc0821a9a20,
0xc0822791d0)
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/github.com/juju/testing/isolation.go:38
+0x44
reflect.Value.call(0xcf2800, 0xc0821a9a20, 0x2213, 0xd242c0, 0x4,
0xc0822adf10, 0x1, 0x1, 0x0, 0x0, ...)
 C:/Go/src/reflect/value.go:435 +0x1214
reflect.Value.Call(0xcf2800, 0xc0821a9a20, 0x2213, 0xc0822adf10, 0x1,
0x1, 0x0, 0x0, 0x0)
 C:/Go/src/reflect/value.go:303 +0xb8
gopkg.in/check%2ev1.(*suiteRunner).runFixture.func1(0xc0822791d0)
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/gopkg.in/check.v1/check.go:721
+0x16a
gopkg.in/check%2ev1.(*suiteRunner).forkCall.func1(0xc0821eac00,
0xc0822791d0, 0xea4fb0)
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/gopkg.in/check.v1/check.go:666
+0x76
created by gopkg.in/check%2ev1.(*suiteRunner).forkCall
 c:/users/admini~1/appdata/local/temp/tmpyrijbo/gogo/src/gopkg.in/check.v1/check.go:667
+0x2a0
FAIL github.com/juju/juju/worker/metrics/sender 1200.101s

But the wait on listener.go:56 only happens after the listener is
closed on listener.go:52. But if the listener is closed, then the
Accept on listener.go:71 will have returned an error and the goroutine
will be in the process of tearing itself down. But we see that isn't
the case, the signal for accept to return with an error has not
happened.

This is usually referred to as concurrent read/close semantics. POSIX
_does_not_ define this behaviour for file descriptors. ie one thread
may be blocked on read, another may issue close but that is not
defined to wake any other thread in a blocking operation on the fd. If
you need this behaviour you must use select(2) or similar.

Go _does_ support this behaviour for network type file descriptors
(anything you get from the net package), which the juju/sockets
packages then wrap. But for the windows version of named pipes
provided by npipe, this behaviour does not hold either because these
operations are not going via select(2), or named pipes on windows do
not support this read/close notification.

This bug needs a windows expert.

Thanks

DAve