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

net: allow not creating a goroutine for netFD.connect in case of caller passes context which is not going to be cancelled manually #49023

Closed
moredure opened this issue Oct 17, 2021 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone

Comments

@moredure
Copy link
Contributor

moredure commented Oct 17, 2021

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

go1.17.2

Does this issue reproduce with the latest release?

Yes

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

GO111MODULE="" GOARCH="amd64" GOBIN="" GOCACHE="/Users/mikefaraponov/Library/Caches/go-build" GOENV="/Users/mikefaraponov/Library/Application Support/go/env" GOEXE="" GOFLAGS="" GOHOSTARCH="amd64" GOHOSTOS="darwin" GOINSECURE="" GOMODCACHE="/Users/mikefaraponov/go/pkg/mod" GONOPROXY="" GONOSUMDB="" GOOS="darwin" GOPATH="/Users/mikefaraponov/go" GOPRIVATE="" GOPROXY="https://proxy.golang.org,direct" GOROOT="/usr/local/Cellar/go/1.17.2/libexec" GOSUMDB="sum.golang.org" GOTMPDIR="" GOTOOLDIR="/usr/local/Cellar/go/go1.17.2/libexec/pkg/tool/darwin_amd64" GCCGO="gccgo" AR="ar" CC="clang" CXX="clang++" CGO_ENABLED="1" GOMOD="/Users/mikefaraponov/go.mod" CGO_CFLAGS="-g -O2" CGO_CPPFLAGS="" CGO_CXXFLAGS="-g -O2" CGO_FFLAGS="-g -O2" CGO_LDFLAGS="-g -O2" PKG_CONFIG="pkg-config" GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/97/d1hwd7t96mj0_jxmpngf2m5w0000gn/T/go-build319243062=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Creating TCPConn connection via net.Dialer

What did you expect to see?

No netFD func2 goroutines to be spawned.

What did you see instead?

Each new connection created create netFD interruptor goroutine

Solution

net/fd_unix.go

if deadline, hasDeadline := ctx.Deadline(); hasDeadline {
  fd.pfd.SetWriteDeadline(deadline)
  defer fd.pfd.SetWriteDeadline(noDeadline)
}

// Start the "interrupter" goroutine, if this context might be canceled.
// (The background context cannot)
//
// The interrupter goroutine waits for the context to be done and
// interrupts the dial (by altering the fd's write deadline, which
// wakes up waitWrite).
if ctxDone := ctx.Done(); ctxDone != nil {
  // skip creation of goroutine and allow fd.pfd.SetWriteDeadline(deadline) to gracefully close this connection on timeout, this allows to pass custom context.Context implementation with deadline but not cancelable or just check some ctx.Value("bypasscreationofgoroutine") instead of ctx.Done() != nil check may be prefereble
  // Wait for the interrupter goroutine to exit before returning
  // from connect.
  done := make(chan struct{})
  interruptRes := make(chan error)
  defer func() {
	  close(done)
	  if ctxErr := <-interruptRes; ctxErr != nil && ret == nil {
		  // The interrupter goroutine called SetWriteDeadline,
		  // but the connect code below had returned from
		  // waitWrite already and did a successful connect (ret
		  // == nil). Because we've now poisoned the connection
		  // by making it unwritable, don't return a successful
		  // dial. This was issue 16523.
		  ret = mapErr(ctxErr)
		  fd.Close() // prevent a leak
	  }
  }()
  go func() {
	  select {
	  case <-ctxDone:
		  // Force the runtime's poller to immediately give up
		  // waiting for writability, unblocking waitWrite
		  // below.
		  fd.pfd.SetWriteDeadline(aLongTimeAgo)
		  testHookCanceledDial()
		  interruptRes <- ctx.Err()
	  case <-done:
		  interruptRes <- nil
	  }
  }()
}

And then smth like this can be used to bypass context cancel mechanism and do not utilise interruptor goroutine:

type deadlineCtx struct {
	deadline time.Time
}

func (m *deadlineCtx) Deadline() (deadline time.Time, ok bool) {
	return m.deadline, true
}

func (m *deadlineCtx) Done() <-chan struct{} {
	return nil
}

func (m *deadlineCtx) Err() error {
	return nil
}

func (m *deadlineCtx) Value(interface{}) interface{} {
	return nil
}
@moredure
Copy link
Contributor Author

Please look #49024

@cherrymui cherrymui changed the title Bypass creation of goroutine for netFD.connect in case of caller passes context which is not going to be cancelled manualy net: allow not creating a goroutine for netFD.connect in case of caller passes context which is not going to be cancelled manually Oct 18, 2021
@cherrymui
Copy link
Member

@moredure thanks for the issue. (For the future) you probably want to mention which package the issue is about and where the code snippet is that you changed. Thanks.

@cherrymui
Copy link
Member

cc @ianlancetaylor

@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 18, 2021
@cherrymui cherrymui added this to the Backlog milestone Oct 18, 2021
@gopherbot
Copy link

Change https://golang.org/cl/356471 mentions this issue: net: use Done rather than comparing with context.Background

@gopherbot
Copy link

Change https://golang.org/cl/372401 mentions this issue: net: start interrupter only if context may be cancelled

@golang golang locked and limited conversation to collaborators Jan 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
None yet
3 participants