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.

Sign up for GitHub

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

Jump to bottom

proposal: net: context variants for udp functions #59897

Open
milkpirate opened this issue Apr 29, 2023 · 5 comments
Open

proposal: net: context variants for udp functions #59897

milkpirate opened this issue Apr 29, 2023 · 5 comments
Labels
Proposal
Milestone
Proposal

Comments

@milkpirate
Copy link

net/udpsock: exposes contexts for several UDP related functions:

In the udpsock package are several (exposed) functions (see below) which use context.Background() as context for calls to unexposed internal functions.

  • ResolveUDPAddr
  • DialUDP
  • ListenUDP
  • ListenMulticastUDP

I propose to expose the contexts to the user. See PR #59880. This makes it possible for the user to hand over its own context and allows hin/her to e.g. cancel or timeout the named UDP actions.

The PR does not change the API for the existing functions above, it just adds another function for each of them with the suffix WithContext and an additional argument (namely the context ctx). This would add the following functions:

  • ResolveUDPAddrWithContext
  • DialUDPWithContext
  • ListenUDPWithContext
  • ListenMulticastUDPWithContext

With the same signature but a context.Conteext added as first argument. Internally the "non-context" function from above, are then just wrappers for the WithContext functions and call them with context.Background().
@milkpirate milkpirate mentioned this issue Apr 29, 2023
Open
@seankhliao seankhliao changed the title Expose contexts for several UDP related functions: net/udpsock proposal: net: context variants for udp functions Apr 29, 2023
@gopherbot gopherbot added this to the Proposal milestone Apr 29, 2023
@seankhliao
Copy link
Member

cc @ianlancetaylor @neild

@ianlancetaylor
Copy link
Contributor

This is largely a dup of #49097, which is a proposal that has been accepted but not yet implemented.

@gopherbot
Copy link

Change https://go.dev/cl/490975 mentions this issue: net: context aware network Dialer.Dial functions

@milkpirate
Copy link
Author

milkpirate commented May 2, 2023 •
edited

@ianlancetaylor
Kinda, at least for DialUDP.

And to be honest, I like my version better (who would have seen that comming?😄), since its:

  • less invasive
  • better to comprehend (since the different functions (with and without context) are closer to each other)
  • so its better to maintain

How do we go forward? Shall I adapt my PR to make context available to other methods/functions in https://go.dev/cl/490975 ?

@ianlancetaylor
Copy link
Contributor

If you think #49097 is making a poor choice of API, then please comment on that issue.

For this issue I suggest that we first complete the work on #49097, then consider whether there is anything to do here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Proposal
Projects
Proposals
Status: Incoming
Milestone
Proposal
Development

No branches or pull requests
5 participants
@ianlancetaylor @gopherbot @milkpirate @seankhliao and others