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

internal/poll: crash setting file completion modes on Windows #22149

Closed
noduluz opened this issue Oct 5, 2017 · 29 comments
Closed

internal/poll: crash setting file completion modes on Windows #22149

noduluz opened this issue Oct 5, 2017 · 29 comments

Comments

@noduluz
Copy link

@noduluz noduluz commented Oct 5, 2017

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

1.9 , 1.9.1

Does this issue reproduce with the latest release?

yes
It works with go 1.8.x in windows 10(64)
Versions 1.8.x and 1.9.x work with linux glibc2.22 (leap42.2)

What operating system and processor architecture are you using (go env)?

set GOARCH=amd64
set GOBIN=
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Users\MZ\go
set GORACE=
set GOROOT=C:\Go
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0
set CXX=g++
set CGO_ENABLED=1
set PKG_CONFIG=pkg-config
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2

What did you do?

compile , run and send a http request

// TestFileServer project main.go
package main

import (
"net/http"
)

func main() {
http.ListenAndServe(":8080", http.FileServer(http.Dir("/temp")))
}

If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

What did you expect to see?

Directory structure or file contents

What did you see instead?

Exception 0xc0000096 0x0 0x0 0x18000890b
PC=0x18000890b

syscall.Syscall(0x7ffce74bbfd0, 0x2, 0x104890, 0x2, 0x0, 0x1e0, 0x66aa20, 0x1)
c:/go/src/runtime/syscall_windows.go:163 +0x69
syscall.SetFileCompletionNotificationModes(0x104890, 0x2, 0x69ae71, 0x3)
c:/go/src/syscall/zsyscall_windows.go:1727 +0x6f
internal/poll.(*FD).Init(0xc042126000, 0x69ae62, 0x3, 0x6ae780, 0x0, 0x0, 0x64)
c:/go/src/internal/poll/fd_windows.go:346 +0xf9
os.newFile(0x104890, 0xc04200e0d0, 0x5, 0x69ae62, 0x3, 0x0)
c:/go/src/os/file_windows.go:57 +0x138
os.openDir(0xc04200e0d0, 0x5, 0x0, 0xc000000000, 0x0)
c:/go/src/os/file_windows.go:145 +0x2d5
os.OpenFile(0xc04200e0d0, 0x5, 0x0, 0x0, 0x2, 0xc04200e0d0, 0x5)
c:/go/src/os/file_windows.go:163 +0x198
os.Open(0xc04200e0d0, 0x5, 0x2, 0xc04200e0d0, 0x5)
c:/go/src/os/file.go:238 +0x4d
net/http.Dir.Open(0x69b166, 0x5, 0xc04200e074, 0x1, 0x0, 0xc042025ba8, 0x402359, 0xc042025bd0)
c:/go/src/net/http/fs.go:75 +0x18a
net/http.(*Dir).Open(0x6cc120, 0xc04200e074, 0x1, 0x0, 0xc042025bf8, 0x4cac74, 0xc042084d80)
:1 +0x64
net/http.serveFile(0x7c48e0, 0xc042104000, 0xc0420fa000, 0x7c1a60, 0x6cc120, 0xc04200e074, 0x1, 0xc042025c01)
c:/go/src/net/http/fs.go:555 +0xbc
net/http.(*fileHandler).ServeHTTP(0xc0420449e0, 0x7c48e0, 0xc042104000, 0xc0420fa000)
c:/go/src/net/http/fs.go:719 +0xb1
net/http.serverHandler.ServeHTTP(0xc042050a90, 0x7c48e0, 0xc042104000, 0xc0420fa000)
c:/go/src/net/http/server.go:2619 +0xbb
net/http.(*conn).serve(0xc042052820, 0x7c4ce0, 0xc042036080)
c:/go/src/net/http/server.go:1801 +0x724
created by net/http.(*Server).Serve
c:/go/src/net/http/server.go:2720 +0x28f

goroutine 1 [IO wait]:
internal/poll.runtime_pollWait(0x12c4f70, 0x72, 0xc042084b58)
c:/go/src/runtime/netpoll.go:173 +0x5e
internal/poll.(*pollDesc).wait(0xc042084c98, 0x72, 0x7bf300, 0x0, 0x0)
c:/go/src/internal/poll/fd_poll_runtime.go:85 +0xb5
internal/poll.(*ioSrv).ExecIO(0x7f5c38, 0xc042084b58, 0xc042152000, 0x17, 0x1, 0x0)
c:/go/src/internal/poll/fd_windows.go:195 +0x13a
internal/poll.(*FD).acceptOne(0xc042084b40, 0x2bc, 0xc04214a000, 0x2, 0x2, 0xc042084b58, 0xc042077c78, 0xc042077c08, 0x410e0f, 0x10)
c:/go/src/internal/poll/fd_windows.go:748 +0xae
internal/poll.(*FD).Accept(0xc042084b40, 0xc042148000, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
c:/go/src/internal/poll/fd_windows.go:782 +0x171
net.(*netFD).accept(0xc042084b40, 0x6aebc8, 0xc042077d90, 0x40245b)
c:/go/src/net/fd_windows.go:192 +0x88
net.(*TCPListener).accept(0xc04207a020, 0x648380, 0xc042077dc0, 0x40112e)
c:/go/src/net/tcpsock_posix.go:136 +0x35
net.(*TCPListener).AcceptTCP(0xc04207a020, 0xc042077e08, 0xc042077e10, 0xc042077e00)
c:/go/src/net/tcpsock.go:234 +0x50
net/http.tcpKeepAliveListener.Accept(0xc04207a020, 0x6ae5a8, 0xc04213c000, 0x7c4da0, 0xc04206e960)
c:/go/src/net/http/server.go:3120 +0x36
net/http.(*Server).Serve(0xc042050a90, 0x7c4960, 0xc04207a020, 0x0, 0x0)
c:/go/src/net/http/server.go:2695 +0x1b9
net/http.(*Server).ListenAndServe(0xc042050a90, 0xc042050a90, 0x404401)
c:/go/src/net/http/server.go:2636 +0xb0
net/http.ListenAndServe(0x69b175, 0x5, 0x7c0e60, 0xc0420449e0, 0x0, 0xc042077f70)
c:/go/src/net/http/server.go:2882 +0x86
main.main()
C:/Users/MZ/Documents/Go/projects/src/TestFileServer/main.go:10 +0x8a

goroutine 4 [IO wait]:
internal/poll.runtime_pollWait(0x12c4eb0, 0x72, 0x0)
c:/go/src/runtime/netpoll.go:173 +0x5e
internal/poll.(*pollDesc).wait(0xc042084ed8, 0x72, 0x7bf300, 0x0, 0x0)
c:/go/src/internal/poll/fd_poll_runtime.go:85 +0xb5
internal/poll.(*ioSrv).ExecIO(0x7f5c38, 0xc042084d98, 0x6ae3e0, 0x0, 0x0, 0x0)
c:/go/src/internal/poll/fd_windows.go:195 +0x13a
internal/poll.(*FD).Read(0xc042084d80, 0xc0420f2071, 0x1, 0x1, 0x0, 0x0, 0x0)
c:/go/src/internal/poll/fd_windows.go:439 +0x266
net.(*netFD).Read(0xc042084d80, 0xc0420f2071, 0x1, 0x1, 0x0, 0x0, 0x0)
c:/go/src/net/fd_windows.go:151 +0x59
net.(*conn).Read(0xc04207a030, 0xc0420f2071, 0x1, 0x1, 0x0, 0x0, 0x0)
c:/go/src/net/net.go:176 +0x74
net/http.(*connReader).backgroundRead(0xc0420f2060)
c:/go/src/net/http/server.go:660 +0x69
created by net/http.(*connReader).startBackgroundRead
c:/go/src/net/http/server.go:656 +0xdf

goroutine 33 [IO wait]:
internal/poll.runtime_pollWait(0x12c4df0, 0x72, 0x0)
c:/go/src/runtime/netpoll.go:173 +0x5e
internal/poll.(*pollDesc).wait(0xc0420f8158, 0x72, 0x7bf300, 0x0, 0x0)
c:/go/src/internal/poll/fd_poll_runtime.go:85 +0xb5
internal/poll.(*ioSrv).ExecIO(0x7f5c38, 0xc0420f8018, 0x6ae3e0, 0x410681, 0xc04212a100, 0x40)
c:/go/src/internal/poll/fd_windows.go:195 +0x13a
internal/poll.(*FD).Read(0xc0420f8000, 0xc04212e000, 0x1000, 0x1000, 0x0, 0x0, 0x0)
c:/go/src/internal/poll/fd_windows.go:439 +0x266
net.(*netFD).Read(0xc0420f8000, 0xc04212e000, 0x1000, 0x1000, 0xc0421098c8, 0x5e0fe2, 0x6741a0)
c:/go/src/net/fd_windows.go:151 +0x59
net.(*conn).Read(0xc042114000, 0xc04212e000, 0x1000, 0x1000, 0x0, 0x0, 0x0)
c:/go/src/net/net.go:176 +0x74
net/http.(*connReader).Read(0xc0421120c0, 0xc04212e000, 0x1000, 0x1000, 0x0, 0x0, 0x0)
c:/go/src/net/http/server.go:753 +0x10c
bufio.(*Reader).fill(0xc04212c000)
c:/go/src/bufio/bufio.go:97 +0x121
bufio.(*Reader).ReadSlice(0xc04212c000, 0xc042109a0a, 0x4101cd, 0x3bb05f0, 0x0, 0xc042109ab8, 0x410681)
c:/go/src/bufio/bufio.go:338 +0x33
bufio.(*Reader).ReadLine(0xc04212c000, 0x100, 0xf8, 0x6911a0, 0x1, 0x12200007f8060, 0xf8)
c:/go/src/bufio/bufio.go:367 +0x3b
net/textproto.(*Reader).readLineSlice(0xc0421120f0, 0xc042109b20, 0xc042109b20, 0x410e0f, 0x100, 0x6911a0)
c:/go/src/net/textproto/reader.go:55 +0x77
net/textproto.(*Reader).ReadLine(0xc0421120f0, 0xc042130000, 0x0, 0xc042109b90, 0x4cb4d9)
c:/go/src/net/textproto/reader.go:36 +0x32
net/http.readRequest(0xc04212c000, 0x0, 0xc042130000, 0x0, 0x0)
c:/go/src/net/http/request.go:925 +0xa0
net/http.(*conn).readRequest(0xc042118000, 0x7c4ce0, 0xc04212a040, 0x0, 0x0, 0x0)
c:/go/src/net/http/server.go:933 +0x183
net/http.(*conn).serve(0xc042118000, 0x7c4ce0, 0xc04212a040)
c:/go/src/net/http/server.go:1739 +0x515
created by net/http.(*Server).Serve
c:/go/src/net/http/server.go:2720 +0x28f

goroutine 49 [IO wait]:
internal/poll.runtime_pollWait(0x12c4d30, 0x72, 0x0)
c:/go/src/runtime/netpoll.go:173 +0x5e
internal/poll.(*pollDesc).wait(0xc042132158, 0x72, 0x7bf300, 0x0, 0x0)
c:/go/src/internal/poll/fd_poll_runtime.go:85 +0xb5
internal/poll.(*ioSrv).ExecIO(0x7f5c38, 0xc042132018, 0x6ae3e0, 0x410681, 0xc04212a240, 0x40)
c:/go/src/internal/poll/fd_windows.go:195 +0x13a
internal/poll.(*FD).Read(0xc042132000, 0xc042150000, 0x1000, 0x1000, 0x0, 0x0, 0x0)
c:/go/src/internal/poll/fd_windows.go:439 +0x266
net.(*netFD).Read(0xc042132000, 0xc042150000, 0x1000, 0x1000, 0xc0421458c8, 0x5e0fe2, 0x6741a0)
c:/go/src/net/fd_windows.go:151 +0x59
net.(*conn).Read(0xc042138000, 0xc042150000, 0x1000, 0x1000, 0x0, 0x0, 0x0)
c:/go/src/net/net.go:176 +0x74
net/http.(*connReader).Read(0xc042112180, 0xc042150000, 0x1000, 0x1000, 0x0, 0x0, 0x0)
c:/go/src/net/http/server.go:753 +0x10c
bufio.(*Reader).fill(0xc04212c060)
c:/go/src/bufio/bufio.go:97 +0x121
bufio.(*Reader).ReadSlice(0xc04212c060, 0xa, 0x0, 0x0, 0x0, 0xc042145ab8, 0x410681)
c:/go/src/bufio/bufio.go:338 +0x33
bufio.(*Reader).ReadLine(0xc04212c060, 0x100, 0xf8, 0x6911a0, 0x4, 0x22004202a380, 0xf8)
c:/go/src/bufio/bufio.go:367 +0x3b
net/textproto.(*Reader).readLineSlice(0xc0421121b0, 0xc042145b20, 0xc042145b20, 0x410e0f, 0x100, 0x6911a0)
c:/go/src/net/textproto/reader.go:55 +0x77
net/textproto.(*Reader).ReadLine(0xc0421121b0, 0xc042130100, 0x0, 0xc042145b90, 0x4cb4d9)
c:/go/src/net/textproto/reader.go:36 +0x32
net/http.readRequest(0xc04212c060, 0x0, 0xc042130100, 0x0, 0x0)
c:/go/src/net/http/request.go:925 +0xa0
net/http.(*conn).readRequest(0xc04213c000, 0x7c4ce0, 0xc04212a180, 0x0, 0x0, 0x0)
c:/go/src/net/http/server.go:933 +0x183
net/http.(*conn).serve(0xc04213c000, 0x7c4ce0, 0xc04212a180)
c:/go/src/net/http/server.go:1739 +0x515
created by net/http.(*Server).Serve
c:/go/src/net/http/server.go:2720 +0x28f
rax 0x0
rbx 0xc04202aa40
rcx 0x104890
rdi 0x3ca000
rsi 0xc0420257f0
rbp 0xc0420257a0
rsp 0x1a0fda0
r8 0x1
r9 0x104890
r10 0x1f
r11 0xbff50
r12 0x0
r13 0xfa
r14 0xfa
r15 0xa
rip 0x18000890b
rflags 0x10246
cs 0x33
fs 0x53
gs 0x2b

@ianlancetaylor ianlancetaylor changed the title http FileServer crash with go 1.9x and Windows 10 (64) internal/poll: crash setting file completion modes on Windows Oct 5, 2017
@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone Oct 5, 2017
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 5, 2017

The stack backtrace suggests that there may be some problem with the setting of hasLoadSetFileCompletionNotificationModes in internal/poll/fd_windows.go.

What version of Windows are you using?

CC @alexbrainman

@noduluz

This comment has been minimized.

Copy link
Author

@noduluz noduluz commented Oct 5, 2017

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Oct 6, 2017

@noduluz

The problem here is that Windows API SetFileCompletionNotificationModes is crashing with ERROR_SYSTEM_TRACE exception (see Exception 0xc0000096 in your stacktrace).

I think this issue is already been reported by many peoples here moby/moby#27423 . Please see if you can follow advise there (it is quite long read unfortunately) to see if it fixes things for you. In summary, you have some software installed on your computer that intercepts SetFileCompletionNotificationModes call, and does not handle it properly. If you uninstall the software, you should be good.

Please close this issue if you confirm my theory. Thank you.

Alex

@noduluz

This comment has been minimized.

Copy link
Author

@noduluz noduluz commented Oct 6, 2017

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Oct 7, 2017

Does this mean that for the moment I have to use 1.8.x and it will be solved in the future for 1.9.x?

I don't plan to change this code. I suggest you read moby/moby#27423 again and follow Microsoft advice. In particular read this moby/moby#27423 (comment) comment (and follow link from the comment) and all following, and you will see that people fixed their problems by uninstalling some buggy products.

Because even if I would know which other application is responsible for intercepting SetFileCompletionModes I can not be sure I’m allowed to so.

SetFileCompletionModes API is part of Windows and it should work and not crash. Talk to your Netadmin and get them to fix SetFileCompletionModes whichever way they like.

The https://support.microsoft.com/en-us/help/2568167/setfilecompletionnotificationmodes-api-causes-an-i-o-completion-port-n describes a way to determine if SetFileCompletionModes is buggy: "... To programmatically determine whether a non-IFS BSP or LSP is installed, enumerate the available protocols by using the WSCEnumerateProtocols function. Then, in each returned WSAPROTOCOL_INFO structure, check the dwServiceFlag1 member to see whether the XP1_IFS_HANDLES flag (0x20000) is set. The Windows SDK documentation for the WSCEnumProtocols function includes source code for an example program that shows how to do this. ...", so we could do that and skip calling SetFileCompletionModes altogether (SetFileCompletionModes is just an optimization). But that might be slow, and I don't want to punish everyone just because of couple computers have buggy software installed.

Alex

@noduluz

This comment has been minimized.

Copy link
Author

@noduluz noduluz commented Oct 8, 2017

@noduluz noduluz closed this Oct 8, 2017
@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Oct 8, 2017

Then let’s hope that the clients I have to install the software on don’t have this kind of a problem.

You can always change Go standard library not to call SetFileCompletionModes Windows API and then compile your software and skip it to your clients. Feel free to ask for help here, if you cannot do it yourself.

Alex

@noduluz

This comment has been minimized.

Copy link
Author

@noduluz noduluz commented Oct 9, 2017

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Oct 10, 2017

@noduluz just for fun I have written little program that prints any 'broken' protocols that you have installed on your system - https://play.golang.org/p/5HEevQM2cL. Can you, please, run it on your system that crashes and post its output here, if you don't mind?

Thank you.

Alex

@noduluz

This comment has been minimized.

Copy link
Author

@noduluz noduluz commented Oct 10, 2017

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Oct 11, 2017

I have the impression that my problem evaporated. My be by the last autoupdate of our virus protection software.

I think you still have a problem. You just get lucky sometimes and you program does not crash.

Broken transport protocols:
LavasoftLSP over [MSAFD Tcpip [TCP/IP]]
LavasoftLSP over [MSAFD Tcpip [UDP/IP]]
LavasoftLSP over [MSAFD Tcpip [TCP/IPv6]]
LavasoftLSP over [MSAFD Tcpip [UDP/IPv6]]

You should have nothing listed under "Broken transport protocols:". I have nothing listed here on my computer. If you look at this link moby/moby#27423 (comment) they even mention same product as yours - "I uninstalled Web Companion by Lavasoft and now docker is working 100% fine.". You should do the same - uninstall "Web Companion by Lavasoft". If you decide to remove it, please run my program again to confirm that "Broken transport protocols:" list is empty.

Thank you.

Alex

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Oct 11, 2017

@noduluz please do not remove "Web Companion by Lavasoft" from your computer. I have some idea I would like you to try before you remove it. I will let you know what to do once I am ready.

Thank you.

Alex

@alexbrainman alexbrainman reopened this Oct 11, 2017
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Oct 11, 2017

Change https://golang.org/cl/69870 mentions this issue: internal/poll: do not call SetFileCompletionNotificationModes if it is broken

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Oct 11, 2017

@noduluz I think I have a solution for you that should work even on your broken computer. Can you, please, try https://go-review.googlesource.com/#/c/go/+/69870 change to see if it fixes your crash? Let me know if you need help building Go with my change.

Thank you

Alex

@noduluz

This comment has been minimized.

Copy link
Author

@noduluz noduluz commented Oct 11, 2017

@as

This comment has been minimized.

Copy link
Contributor

@as as commented Oct 11, 2017

@noduluz
git clone https://go.googlesource.com/go
cd go
git checkout 27366157ed762f687516df2ae4514f789b44b213

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Oct 12, 2017

@noduluz
You need to be able to build Go from source. There is https://golang.org/doc/install/source for that.
Once you could do that, you need to get particular revision of Go source. - the command for that is:

git fetch https://go.googlesource.com/go refs/changes/70/69870/2 && git checkout FETCH_HEAD

(I got the comand by going to https://go-review.googlesource.com/#/c/go/+/69870/ then clicked "Download" in the right top conner, and copied line titled with Checkout into my clipboard)

If you cannot verify, that is fine.

Alex

@gopherbot gopherbot closed this in c37647f Oct 12, 2017
@noduluz

This comment has been minimized.

Copy link
Author

@noduluz noduluz commented Oct 12, 2017

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Oct 12, 2017

@noduluz
FYI I just submitted the change:

commit c37647fc3abd3aa9307d5abbce63c9d6c7c70602
Author: Alex Brainman <alex.brainman@gmail.com>
Date:   Wed Oct 11 18:15:25 2017 +1100

    internal/poll: do not call SetFileCompletionNotificationModes if it is broken
    
    Current code assumes that SetFileCompletionNotificationModes
    is safe to call even if we know that it is not safe to use
    FILE_SKIP_COMPLETION_PORT_ON_SUCCESS flag. It appears (see issue #22149),
    SetFileCompletionNotificationModes crashes when we call it without
    FILE_SKIP_COMPLETION_PORT_ON_SUCCESS flag.
    
    Do not call SetFileCompletionNotificationModes in that situation.
    We are allowed to do that, because SetFileCompletionNotificationModes
    is just an optimisation.
    
    Fixes #22149
    
    Change-Id: I0ad3aff4eabd8c27739417a62c286b1819ae166a
    Reviewed-on: https://go-review.googlesource.com/69870
    Reviewed-by: Ian Lance Taylor <iant@golang.org>

So you can find it in the main tree now.

Alex

@noduluz

This comment has been minimized.

Copy link
Author

@noduluz noduluz commented Oct 12, 2017

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Oct 12, 2017

I compiled version:
go version devel +862fb86c6e Thu Oct 12 07:13:23 2017 +0000 windows/amd64
and it seems to work perfectly. Everything concerning this issue compiles & runs as it should.

Sounds good. Thank you for confirming.

Alex

@noduluz

This comment has been minimized.

Copy link
Author

@noduluz noduluz commented Oct 12, 2017

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Oct 12, 2017

Then I can close this thread?

It is already closed. Our automatic gopherbot closed the issue when I submitted my change.
All is good. Thank you for helping.

Alex

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Oct 15, 2017

@alexbrainman, two questions:

  1. Do you think this change is safe to include in Go 1.9.2?
  2. Do you think this change is important to include in Go 1.9.2?

Including it will make the fix to #22024 apply cleanly, but it looks like maybe it's serious enough to warrant inclusion on its own merits too. I am going to assume yes we can and should include it, but please confirm or push back.

Thanks.

@rsc rsc modified the milestones: Go1.10, Go1.9.2 Oct 15, 2017
@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Oct 15, 2017

CL 69870 OK for Go 1.9.2.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Oct 15, 2017

Change https://golang.org/cl/70989 mentions this issue: [release-branch.go1.9] internal/poll: do not call SetFileCompletionNotificationModes if it is broken

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Oct 15, 2017

I assumed you are asking about CL 69870
internal/poll: do not call SetFileCompletionNotificationModes if it is broken

Do you think this change is safe to include in Go 1.9.2?

CL 69870 is safe because it avoids calling crashing SetFileCompletionNotificationModes Windows API (SetFileCompletionNotificationModes is just an optimization, so it can be skipped). On the other hand CL 69870 is fairly large, but I looked at it again and it LGTM. CL 69870 is also fairly new and has not been tested by Go users.

Do you think this change is important to include in Go 1.9.2?

I don't think it is important to include CL 69870. I feel it is fairly rare to find computer with crashing SetFileCompletionNotificationModes. No one complained about it yet, but it was part of our code for awhile.

Including it will make the fix to #22024 apply cleanly, but it looks like maybe it's serious enough to warrant inclusion on its own merits too. I am going to assume yes we can and should include it, but please confirm or push back.

I think it is fine to include CL 69870, just to make CL 69871 apply cleanly. Sorry I should have submitted CL 69871 first.

Alex

@AnyCPU

This comment has been minimized.

Copy link

@AnyCPU AnyCPU commented Oct 24, 2017

@rsc I think if you include this change in Go 1.9.2, everyone who uses the VMware vSockets (dgram & stream) will thank you.

gopherbot pushed a commit that referenced this issue Oct 25, 2017
…tificationModes if it is broken

Current code assumes that SetFileCompletionNotificationModes
is safe to call even if we know that it is not safe to use
FILE_SKIP_COMPLETION_PORT_ON_SUCCESS flag. It appears (see issue #22149),
SetFileCompletionNotificationModes crashes when we call it without
FILE_SKIP_COMPLETION_PORT_ON_SUCCESS flag.

Do not call SetFileCompletionNotificationModes in that situation.
We are allowed to do that, because SetFileCompletionNotificationModes
is just an optimisation.

Fixes #22149

Change-Id: I0ad3aff4eabd8c27739417a62c286b1819ae166a
Reviewed-on: https://go-review.googlesource.com/69870
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/70989
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Oct 26, 2017

go1.9.2 has been packaged and includes:

The release is posted at golang.org/dl.

— golang.org/x/build/cmd/releasebot, Oct 26 21:09:20 UTC

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.