New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 699182 link

Starred by 1 user
Status: Fixed
Owner:
d...@chromium.org
Closed: Mar 2017
Cc:
█ iannucci@chromium.org
no...@chromium.org
estaab@chromium.org
hinoka@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Race in "npipe" package causing unit test race thanks to new unit test.

Project Member Reported by d...@chromium.org, Mar 7 2017 
In LUCI repository, I added a new test that exercised the "streamserver" Windows package. This revealed an implementation race in the "npipe" package that is now clogging up the system.

Fortunately, since "npipe" was released, Microsoft released an official Go Windows I/O library: https://chromium.googlesource.com/external/github.com/Microsoft/go-winio

Replacing "npipe" with this library removes the race and is probably healthier all-around.
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 7 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/b1fb723fc4ce6d0b8167ecf4cac327386380d74b

commit b1fb723fc4ce6d0b8167ecf4cac327386380d74b
Author: Dan Jacques <dnj@chromium.org>
Date: Tue Mar 07 19:23:20 2017

Bump Go deps, add "Microsoft/go-winio".

After the next LUCI roll, we can remove "npipe" package from the Go deps
in favor of "go-winio".

BUG= chromium:699182 
TEST=None

Change-Id: I76dd12cb7c9f61d8c0c7db7eae503bb0d1a21bed
Reviewed-on: https://chromium-review.googlesource.com/450784
Commit-Queue: Daniel Jacques <dnj@chromium.org>
Reviewed-by: Nodir Turakulov <nodir@chromium.org>

[modify] https://crrev.com/b1fb723fc4ce6d0b8167ecf4cac327386380d74b/go/deps.lock
[modify] https://crrev.com/b1fb723fc4ce6d0b8167ecf4cac327386380d74b/go/deps.yaml
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 7 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-go.git/+/e4105d6bb5c671411e291e6e4f02d98634a71326

commit e4105d6bb5c671411e291e6e4f02d98634a71326
Author: dnj <dnj@chromium.org>
Date: Tue Mar 07 20:31:40 2017

Disable Windows npipe test b/c of flake.

The test is flaky. This will be re-enabled shortly.

TBR=vadimsh@chromium.org
BUG= chromium:699182 
TEST=None

Review-Url: https://codereview.chromium.org/2734133004

[modify] https://crrev.com/e4105d6bb5c671411e291e6e4f02d98634a71326/logdog/client/butler/streamserver/namedPipe_windows_test.go
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 7 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-go.git/+/0061578294f73ba706c26bc6cf16ba8602e67d82

commit 0061578294f73ba706c26bc6cf16ba8602e67d82
Author: dnj <dnj@chromium.org>
Date: Tue Mar 07 20:40:23 2017

LogDog: Use "winio" instead of "npipe".

This should fix a pretty bad flake in Windows stream server testing.
This is also a good health move, since "winio" is an official Microsoft
library!

BUG= chromium:699182 
TEST=dev
  - Ran this on a development Windows machine, seems to be reliably
    working.

R=nodir@chromium.org

Review-Url: https://codereview.chromium.org/2733243003

[modify] https://crrev.com/0061578294f73ba706c26bc6cf16ba8602e67d82/logdog/client/butler/streamserver/namedPipe_windows.go
[modify] https://crrev.com/0061578294f73ba706c26bc6cf16ba8602e67d82/logdog/client/butler/streamserver/namedPipe_windows_test.go
[modify] https://crrev.com/0061578294f73ba706c26bc6cf16ba8602e67d82/logdog/client/butlerlib/streamclient/client_namedPipe_windows.go

Comment 4 by d...@chromium.org, Mar 7 2017

Status: Fixed (was: Started)
Yarp, should be fixed now.

Comment 5 by estaab@chromium.org, Mar 7 2017 

Awesome! Nice work finding this and fixing a bug in the process.
►

Sign in to add a comment