From 5fdbdcc2ae1c7e1073157fa7cb34a15eab472e1d Mon Sep 17 00:00:00 2001 From: Simeone Date: Tue, 26 Mar 2019 09:43:43 +0100 Subject: [PATCH 1/6] add DialPipeContext --- pipe.go | 48 +++++++++++++++++++++++++++++++++--------------- pipe_test.go | 21 +++++++++++++++++++++ 2 files changed, 54 insertions(+), 15 deletions(-) diff --git a/pipe.go b/pipe.go index d99eedb..d35a94e 100644 --- a/pipe.go +++ b/pipe.go @@ -3,6 +3,7 @@ package winio import ( + "context" "errors" "io" "net" @@ -137,9 +138,30 @@ func (s pipeAddress) String() string { return string(s) } +func tryDialPipe(ctx context.Context, path *string) (syscall.Handle, error) { + for { + select { + case <-ctx.Done(): + err := ctx.Err() + if err == context.DeadlineExceeded { + err = ErrTimeout + } else { + err = &os.PathError{Op: "open", Path: *path, Err: err} + } + return syscall.Handle(0), err + default: + h, err := createFile(*path, syscall.GENERIC_READ|syscall.GENERIC_WRITE, 0, nil, syscall.OPEN_EXISTING, syscall.FILE_FLAG_OVERLAPPED|cSECURITY_SQOS_PRESENT|cSECURITY_ANONYMOUS, 0) + if err != cERROR_PIPE_BUSY { + return h, &os.PathError{Op: "open", Path: *path, Err: err} + } + time.Sleep(time.Millisecond * 10) + } + } +} + // DialPipe connects to a named pipe by path, timing out if the connection // takes longer than the specified duration. If timeout is nil, then we use -// a default timeout of 5 seconds. (We do not use WaitNamedPipe.) +// a default timeout of 2 seconds. (We do not use WaitNamedPipe.) func DialPipe(path string, timeout *time.Duration) (net.Conn, error) { var absTimeout time.Time if timeout != nil { @@ -147,23 +169,19 @@ func DialPipe(path string, timeout *time.Duration) (net.Conn, error) { } else { absTimeout = time.Now().Add(time.Second * 2) } + ctx, _ := context.WithDeadline(context.Background(), absTimeout) + conn, err := DialPipeContext(ctx, path) + return conn, err +} + +//DialPipeContext connects to a named pipe. ctx can be used to cancel or +//expire the pending connection ( We do not use WaitNamedPipe.) +func DialPipeContext(ctx context.Context, path string) (net.Conn, error) { var err error var h syscall.Handle - for { - h, err = createFile(path, syscall.GENERIC_READ|syscall.GENERIC_WRITE, 0, nil, syscall.OPEN_EXISTING, syscall.FILE_FLAG_OVERLAPPED|cSECURITY_SQOS_PRESENT|cSECURITY_ANONYMOUS, 0) - if err != cERROR_PIPE_BUSY { - break - } - if time.Now().After(absTimeout) { - return nil, ErrTimeout - } - - // Wait 10 msec and try again. This is a rather simplistic - // view, as we always try each 10 milliseconds. - time.Sleep(time.Millisecond * 10) - } + h, err = tryDialPipe(ctx, &path) if err != nil { - return nil, &os.PathError{Op: "open", Path: path, Err: err} + return nil, err } var flags uint32 diff --git a/pipe_test.go b/pipe_test.go index 3869207..d69ceb4 100644 --- a/pipe_test.go +++ b/pipe_test.go @@ -3,6 +3,7 @@ package winio import ( "bufio" "bytes" + "context" "io" "net" "os" @@ -37,6 +38,26 @@ func TestDialListenerTimesOut(t *testing.T) { } } +func TestDialListenerGetsCancelled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + l, err := ListenPipe(testPipeName, nil) + if err != nil { + t.Fatal(err) + } + ch := make(chan error) + defer l.Close() + go func(ctx context.Context, ch chan error) { + _, err := DialPipeContext(ctx, testPipeName) + ch <- err + }(ctx, ch) + time.Sleep(time.Millisecond * 30) + cancel() + err = <-ch + if err == nil { + t.Fatalf("expected ErrTimeout, got %v", err) + } +} + func TestDialAccessDeniedWithRestrictedSD(t *testing.T) { c := PipeConfig{ SecurityDescriptor: "D:P(A;;0x1200FF;;;WD)", From b9aaa11ea0d8c7976b63bb36426f6cd21b6290cc Mon Sep 17 00:00:00 2001 From: Simeone Date: Tue, 26 Mar 2019 10:07:58 +0100 Subject: [PATCH 2/6] errors coercion to pass the tests --- pipe.go | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/pipe.go b/pipe.go index d35a94e..33087df 100644 --- a/pipe.go +++ b/pipe.go @@ -138,22 +138,22 @@ func (s pipeAddress) String() string { return string(s) } +//helper function used to try to open the pipe multiple times. func tryDialPipe(ctx context.Context, path *string) (syscall.Handle, error) { for { select { case <-ctx.Done(): - err := ctx.Err() - if err == context.DeadlineExceeded { - err = ErrTimeout - } else { - err = &os.PathError{Op: "open", Path: *path, Err: err} - } - return syscall.Handle(0), err + return syscall.Handle(0), ctx.Err() default: h, err := createFile(*path, syscall.GENERIC_READ|syscall.GENERIC_WRITE, 0, nil, syscall.OPEN_EXISTING, syscall.FILE_FLAG_OVERLAPPED|cSECURITY_SQOS_PRESENT|cSECURITY_ANONYMOUS, 0) - if err != cERROR_PIPE_BUSY { - return h, &os.PathError{Op: "open", Path: *path, Err: err} + if err == nil { + return h, nil } + if err != cERROR_PIPE_BUSY { + return h, newOpenError(path, err) + } + // Wait 10 msec and try again. This is a rather simplistic + // view, as we always try each 10 milliseconds. time.Sleep(time.Millisecond * 10) } } @@ -171,9 +171,19 @@ func DialPipe(path string, timeout *time.Duration) (net.Conn, error) { } ctx, _ := context.WithDeadline(context.Background(), absTimeout) conn, err := DialPipeContext(ctx, path) + if err == context.DeadlineExceeded { + return nil, ErrTimeout + } return conn, err } +func newOpenError(path *string, err error) error { + if err != nil { + return &os.PathError{Err: err, Op: "open", Path: *path} + } + return nil +} + //DialPipeContext connects to a named pipe. ctx can be used to cancel or //expire the pending connection ( We do not use WaitNamedPipe.) func DialPipeContext(ctx context.Context, path string) (net.Conn, error) { From 665a579a64b3e14b6bad14d83bef1aac191dac59 Mon Sep 17 00:00:00 2001 From: Simeone Date: Tue, 26 Mar 2019 10:10:46 +0100 Subject: [PATCH 3/6] test context cancel error type --- pipe_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pipe_test.go b/pipe_test.go index d69ceb4..964cadc 100644 --- a/pipe_test.go +++ b/pipe_test.go @@ -53,8 +53,8 @@ func TestDialListenerGetsCancelled(t *testing.T) { time.Sleep(time.Millisecond * 30) cancel() err = <-ch - if err == nil { - t.Fatalf("expected ErrTimeout, got %v", err) + if err != context.Canceled { + t.Fatalf("expected context.Canceled, got %v", err) } } From 693f74258b9d20862052c84492b2e69187b1b39c Mon Sep 17 00:00:00 2001 From: sime1 Date: Tue, 26 Mar 2019 18:36:36 +0100 Subject: [PATCH 4/6] context timeout test --- pipe_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pipe_test.go b/pipe_test.go index 964cadc..89e32d1 100644 --- a/pipe_test.go +++ b/pipe_test.go @@ -38,6 +38,20 @@ func TestDialListenerTimesOut(t *testing.T) { } } +func TestDialContextListenerTimesOut(t *testing.T) { + l, err := ListenPipe(testPipeName, nil) + if err != nil { + t.Fatal(err) + } + defer l.Close() + var d = time.Duration(10 * time.Millisecond) + ctx, _ := context.WithTimeout(context.Background(), d) + _, err = DialPipeContext(ctx, testPipeName) + if err != context.DeadlineExceeded { + t.Fatalf("expected context.DeadlineExceeded, got %v", err) + } +} + func TestDialListenerGetsCancelled(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) l, err := ListenPipe(testPipeName, nil) From 498f830763968a52d6ffb6731ca228e4354aec14 Mon Sep 17 00:00:00 2001 From: sime1 Date: Tue, 26 Mar 2019 18:37:37 +0100 Subject: [PATCH 5/6] formatting, inline Dial returned error --- pipe.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pipe.go b/pipe.go index 33087df..75b8b74 100644 --- a/pipe.go +++ b/pipe.go @@ -138,7 +138,7 @@ func (s pipeAddress) String() string { return string(s) } -//helper function used to try to open the pipe multiple times. +// tryDialPipe attempts to dial the pipe at `path` until `ctx` cancellation or timeout. func tryDialPipe(ctx context.Context, path *string) (syscall.Handle, error) { for { select { @@ -150,7 +150,7 @@ func tryDialPipe(ctx context.Context, path *string) (syscall.Handle, error) { return h, nil } if err != cERROR_PIPE_BUSY { - return h, newOpenError(path, err) + return h, &os.PathError{Err: err, Op: "open", Path: *path} } // Wait 10 msec and try again. This is a rather simplistic // view, as we always try each 10 milliseconds. @@ -184,8 +184,8 @@ func newOpenError(path *string, err error) error { return nil } -//DialPipeContext connects to a named pipe. ctx can be used to cancel or -//expire the pending connection ( We do not use WaitNamedPipe.) +// DialPipeContext attempts to connect to a named pipe by `path` until `ctx` +// cancellation or timeout. func DialPipeContext(ctx context.Context, path string) (net.Conn, error) { var err error var h syscall.Handle From 6e309c35848b930aefd9a663fa11fd0301e714df Mon Sep 17 00:00:00 2001 From: sime1 Date: Tue, 26 Mar 2019 18:39:51 +0100 Subject: [PATCH 6/6] remove newOpenError --- pipe.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/pipe.go b/pipe.go index 75b8b74..eaa1523 100644 --- a/pipe.go +++ b/pipe.go @@ -177,13 +177,6 @@ func DialPipe(path string, timeout *time.Duration) (net.Conn, error) { return conn, err } -func newOpenError(path *string, err error) error { - if err != nil { - return &os.PathError{Err: err, Op: "open", Path: *path} - } - return nil -} - // DialPipeContext attempts to connect to a named pipe by `path` until `ctx` // cancellation or timeout. func DialPipeContext(ctx context.Context, path string) (net.Conn, error) {