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 RawConn.Control to get fd instead of Fd() #3

Merged
merged 1 commit into from
Nov 4, 2020

Conversation

LinkShen
Copy link

@LinkShen LinkShen commented Nov 4, 2020

file.Fd() has a potential bug when work with unblocking connection which is the default setting of Golang lib.

// Fd returns the integer Unix file descriptor referencing the open file.
// The file descriptor is valid only until f.Close is called or f is garbage collected.
// On Unix systems this will cause the SetDeadline methods to stop working.
func (f *File) Fd() uintptr {
	if f == nil {
		return ^(uintptr(0))
	}

	// If we put the file descriptor into nonblocking mode,
	// then set it to blocking mode before we return it,
	// because historically we have always returned a descriptor
	// opened in blocking mode. The File will continue to work,
	// but any blocking operation will tie up a thread.
	if f.nonblock {
		f.pfd.SetBlocking() // HERE!!!
	}

	return uintptr(f.pfd.Sysfd)
}

This will case tcpconn becoming blocking, and may case goroutine blocked at conn.Close().
check this link for more information: golang/go#29277

So i use RawConn.Control which used in many Golang lib to do the syscall.

@brucespang
Copy link
Owner

Looks good to me, thank you for the patch!

@brucespang brucespang merged commit 2d09013 into brucespang:master Nov 4, 2020
yurishkuro pushed a commit to jaegertracing/jaeger that referenced this pull request May 11, 2023
<!--
Please delete this comment before posting.

We appreciate your contribution to the Jaeger project! 👋🎉

Before creating a pull request, please make sure:
- Your PR is solving one problem
- You have read the guide for contributing
- See
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING.md
- You signed all your commits (otherwise we won't be able to merge the
PR)
- See
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md#certificate-of-origin---sign-your-work
- You added unit tests for the new functionality
- You mention in the PR description which issue it is addressing, e.g.
"Resolves #123"
-->

## Which problem is this PR solving?
Resolves #4448

## Short description of the changes
Jaeger agent gets stuck when closing with SocketBufferSize set. This is
because `Close()` of `net.UDPConn` will be blocked if `Fd()` is used to
get the file descriptor.
Use `RawConn.Control` instead to get fd to set the socket buffer.

Same issue was discussed here: golang/go#29277
The fix refers to here: brucespang/go-tcpinfo#3

Signed-off-by: Chen Xu <chen.x@uber.com>
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.

2 participants