Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use our fork of github.com/Microsoft/go-winio #13745

Merged
merged 2 commits into from
Sep 14, 2018

Conversation

zanderz
Copy link
Contributor

@zanderz zanderz commented Sep 13, 2018

There was still a problem after all with the named pipe lib, and CLI commands which call FixVersionClash(). The problem is where FixVersionClash() calls Close() on the socket, while the client transport is still sitting in a read loop. The pipe library returns a scary error, which gets printed out in red:
- ERROR {pipe://\\.\pipe\kbservice\Users\Steve\AppData\Local\Keybase\keybased.sock} Error in transport: msgpack decode error [pos 3]: file has already been closed

Made a fork of microsoft/go-winio with this PR: microsoft/go-winio#94

Call stacks:

github.com/keybase/client/go/vendor/github.com/Microsoft/go-winio.(*win32File).asyncIo (c:\work\src\github.com\keybase\client\go\vendor\github.com\Microsoft\go-winio\file.go:191)
github.com/keybase/client/go/vendor/github.com/Microsoft/go-winio.(*win32File).Read (c:\work\src\github.com\keybase\client\go\vendor\github.com\Microsoft\go-winio\file.go:224)
bufio.(*Reader).Read (c:\Go\src\bufio\bufio.go:216)
github.com/keybase/client/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc.(*lastErrReader).Read (c:\work\src\github.com\keybase\client\go\vendor\github.com\keybase\go-framed-msgpack-rpc\rpc\packetizer.go:23)
github.com/keybase/client/go/vendor/github.com/keybase/go-codec/codec.(*ioDecReader).Read (c:\work\src\github.com\keybase\client\go\vendor\github.com\keybase\go-codec\codec\decode.go:600)
github.com/keybase/client/go/vendor/github.com/keybase/go-codec/codec.(*ioDecReader).ReadByte (c:\work\src\github.com\keybase\client\go\vendor\github.com\keybase\go-codec\codec\decode.go:615)
github.com/keybase/client/go/vendor/github.com/keybase/go-codec/codec.(*ioDecReader).readn1 (c:\work\src\github.com\keybase\client\go\vendor\github.com\keybase\go-codec\codec\decode.go:692)
github.com/keybase/client/go/vendor/github.com/keybase/go-codec/codec.(*msgpackDecDriver).readNextBd (c:\work\src\github.com\keybase\client\go\vendor\github.com\keybase\go-codec\codec\msgpack.go:710)
github.com/keybase/client/go/vendor/github.com/keybase/go-codec/codec.(*msgpackDecDriver).TryDecodeAsNil (c:\work\src\github.com\keybase\client\go\vendor\github.com\keybase\go-codec\codec\msgpack.go:748)
github.com/keybase/client/go/vendor/github.com/keybase/go-codec/codec.(*Decoder).MustDecode (c:\work\src\github.com\keybase\client\go\vendor\github.com\keybase\go-codec\codec\decode.go:2058)
github.com/keybase/client/go/vendor/github.com/keybase/go-codec/codec.(*Decoder).Decode (c:\work\src\github.com\keybase\client\go\vendor\github.com\keybase\go-codec\codec\decode.go:2047)
github.com/keybase/client/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc.(*packetHandler).loadNextFrame (c:\work\src\github.com\keybase\client\go\vendor\github.com\keybase\go-framed-msgpack-rpc\rpc\packetizer.go:97)
github.com/keybase/client/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc.(*packetHandler).NextFrame (c:\work\src\github.com\keybase\client\go\vendor\github.com\keybase\go-framed-msgpack-rpc\rpc\packetizer.go:64)
github.com/keybase/client/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc.(*transport).receiveFramesLoop (c:\work\src\github.com\keybase\client\go\vendor\github.com\keybase\go-framed-msgpack-rpc\rpc\transport.go:155)
runtime.goexit (c:\Go\src\runtime\asm_amd64.s:2361)



github.com/keybase/client/go/vendor/github.com/Microsoft/go-winio.(*win32File).closeHandle (c:\work\src\github.com\keybase\client\go\vendor\github.com\Microsoft\go-winio\file.go:123)
github.com/keybase/client/go/vendor/github.com/Microsoft/go-winio.(*win32File).Close (c:\work\src\github.com\keybase\client\go\vendor\github.com\Microsoft\go-winio\file.go:134)
github.com/keybase/client/go/client.FixVersionClash.func1 (c:\work\src\github.com\keybase\client\go\client\versionfix.go:63)
github.com/keybase/client/go/client.FixVersionClash (c:\work\src\github.com\keybase\client\go\client\versionfix.go:114)
main.configureProcesses (c:\work\src\github.com\keybase\client\go\keybase\main.go:298)
main.mainInner (c:\work\src\github.com\keybase\client\go\keybase\main.go:190)
main.main (c:\work\src\github.com\keybase\client\go\keybase\main.go:75)
runtime.main (c:\Go\src\runtime\proc.go:198)
runtime.goexit (c:\Go\src\runtime\asm_amd64.s:2361)

@zanderz zanderz requested review from maxtaco, akalin-keybase and a team September 13, 2018 22:38
Copy link
Contributor

@akalin-keybase akalin-keybase left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we need to update imports of go-winio to point to our fork?

Copy link
Contributor

@akalin-keybase akalin-keybase left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@zanderz
Copy link
Contributor Author

zanderz commented Sep 13, 2018

yeah I forgot to change the imports at first

@maxtaco
Copy link
Contributor

maxtaco commented Sep 24, 2018

@zanderz is #13868 the same issue? Does2.6.2 have the fix?

@maxtaco
Copy link
Contributor

maxtaco commented Sep 24, 2018

Ah, I seem to recall the discussion, we deemed it too big...

@zanderz
Copy link
Contributor Author

zanderz commented Sep 24, 2018

I wanted to put it in the hotfix but nobody else seemed to.

@maxtaco
Copy link
Contributor

maxtaco commented Sep 24, 2018

Makes sense. We'll wait until the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants