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: document Dial thread safety, timeout example #33743

Closed
kevinburke1 opened this issue Aug 20, 2019 · 5 comments
Closed

net: document Dial thread safety, timeout example #33743

kevinburke1 opened this issue Aug 20, 2019 · 5 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@kevinburke1
Copy link

kevinburke1 commented Aug 20, 2019

I'm trying to figure out whether port 80 on the IP address 169.254.169.254 is reachable from the host I'm on. Essentially all I want to do is connect to it with a short timeout. I'm a little confused by the net package on this point.

  • Generally speaking I thought timeouts were deprecated in favor of using a context with a timeout parameter. However, the net package still has a DialTimeout top level function and no DialContext.
  • There is a (*Dialer) DialContext() method. After some reading I see that the empty Dialer is equivalent to calling net.Dial, there's no DefaultDialer or similar. However, is a Dialer safe to use concurrently? (eg. If I am going to use DialContext, can I just have one package-level Dialer or similar, instead of allocating a new one each time.) It's not clear from the documentation.

It might be good to have some examples here, some guidance about when to use Context vs. Timeout, and a short note about whether a Dialer is safe for concurrent use.

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 20, 2019
@bcmills bcmills added this to the Go1.14 milestone Aug 20, 2019
@bcmills
Copy link
Contributor

bcmills commented Aug 20, 2019

CC @mikioh @ianlancetaylor

@ianlancetaylor
Copy link
Contributor

It's safe to call Dialer methods concurrently. It's of course not safe to change Dialer fields concurrently or concurrently with method calls. I agree that this should be documented.

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 20, 2019
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 20, 2019
@tomocy
Copy link
Contributor

tomocy commented Aug 23, 2019

Can I tackle on this??

Sent with GitHawk

@ianlancetaylor
Copy link
Contributor

@tomocy Sure. Thanks.

tomocy added a commit to tomocy/go that referenced this issue Aug 27, 2019
The current code has no example of Dialer, so add an example to show how and when Dialer.Timeout and (*Dialer).DialContext are used.

Fixes golang#33743
tomocy added a commit to tomocy/go that referenced this issue Aug 27, 2019
The existing code does not mention the concurrent safety of a Dialer, so add short note that it is safe to call Dialer methods concurrently.

Fixes golang#33743
tomocy added a commit to tomocy/go that referenced this issue Aug 27, 2019
The current code has no example of Dialer, so add an example to show how and when Dialer.Timeout and (*Dialer).DialContext are used.

Fixes golang#33743
tomocy added a commit to tomocy/go that referenced this issue Aug 27, 2019
The existing code does not mention the concurrent safety of a Dialer, so add short note that it is safe to call Dialer methods concurrently.

Fixes golang#33743
@gopherbot
Copy link

Change https://golang.org/cl/191879 mentions this issue: net: add example of Dialer

tomocy added a commit to tomocy/go that referenced this issue Sep 1, 2019
The existing code does not mention the concurrent safety of a Dialer, so add short note that it is safe to call Dialer methods concurrently.

Fixes golang#33743
tomocy added a commit to tomocy/go that referenced this issue Sep 2, 2019
The previous example was unusual because it used (*Dialer).Dial in a goroutine differennt frrom main one and because it is not necessary to receive Conn from the channel since Conn is produced only once, so modify the example to show just how (*Dialer).DialContext is used.

Updates golang#33743
tomocy added a commit to tomocy/go that referenced this issue Sep 2, 2019
The previous example was unusual because it used (*Dialer).Dial in a goroutine differennt frrom main one and because it is not necessary to receive Conn from the channel since Conn is produced only once, so modify the example to show just how (*Dialer).DialContext is used.

Updates golang#33743
tomocy added a commit to tomocy/go that referenced this issue Sep 2, 2019
The previous example was unusual because it used (*Dialer).Dial in a goroutine differennt frrom main one and because it is not necessary to receive Conn from the channel since Conn is produced only once, so modify the example to show just how (*Dialer).DialContext is used.

Updates golang#33743
tomocy added a commit to tomocy/go that referenced this issue Sep 2, 2019
The previous example was unusual because it used (*Dialer).Dial in a goroutine differennt frrom main one and because it is not necessary to receive Conn from the channel since Conn is produced only once, so modify the example to show just how (*Dialer).DialContext is used.

Updates golang#33743
t4n6a1ka pushed a commit to t4n6a1ka/go that referenced this issue Sep 5, 2019
Fixes golang#33743.

Change-Id: I80621321d56b6cf312a86e272800f1ad03c5544c
GitHub-Last-Rev: d91cb36
GitHub-Pull-Request: golang#33856
Reviewed-on: https://go-review.googlesource.com/c/go/+/191879
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
@golang golang locked and limited conversation to collaborators Sep 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants