Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings
/goPublic

proposal: x/sync/errgroup: pass errgroup.WithContext's derived context directly #34510

Open
Labels
Milestone
@kylelemons

Description

@kylelemons

Since API changes are something that is now possible to do with module versions, I thought it would be worth mentioning one that gnaws at me pretty frequently.

We use contexts heavily in my code-base, and it comes up moderately often that we want a cancellable or deadline-respectingsync.WaitGroup, and for this purpose authors tend to turn tox/sync/errgroup. This often works out fine as long as the scope of theerrgroup.Group is confined to a single function, but it also regularly will be used with something somewhat stateful. These frequently follow a certain pattern, the core elements of which are exemplified by this contrived example:

typeServerRunnerstruct {group*errgroup.Group// *Group as a fieldgroupCtx context.Context// context as a field (heavily discouraged)}func (sr*ServiceRunner)Run(server*servers.Server) {sr.Group.Go(func()error {returnserver.Run(sr.groupCtx) })// wrapper that calls Go}func (sr*ServiceRunner)Wait(ctx context.Context) {select {case<-ctx.Done():returnctx.Err()case<-sr.groupCtx.Done():sr.groupCtx.Wait()// wrapper that calls wait and/or ctx.Donereturnsr.groupCtx.Err()  }}

I assert (without evidence) that this boilerplate is pretty common among context-respecting code that interacts witherrgroup.Group, and in fact the above could be almost directly converted into a utility package, but then we would have ContextGroup wrapping errgroup wrapping WaitGroup... which feels excessive.

I have also observed that the context returned byWithContext is occasionally misused for code other than the goroutines spawned byGo, in some cases by directly shadowing thectx variable, which often results in spooky action at a distance where one failure causes code in another part of the application to have its context cancelled.

So, I propose that the API for errgroup split out the context- and non-context APIs:

typeGroupstruct {/* ... */ }func (Group)Go(func()error) {/* ... */ }func (Group)Wait()error {/* ... */ }typeContextGroupstruct {/* ... */ }funcWithContext(ctx context.Context)*ContextGroup {/* ... */ }func (ContextGroup)Go(func(context.Context)error) {/* ... */ }func (ContextGroup)Wait(context.Context)error {/* ... */ }

Unfortunately, this is definitely a backward-incompatible change, and one for which there is probably little chance for a mechanical rewrite unless ContextGroup had a mechanism for retrieving its context.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    Status

    Incoming

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions


      [8]ページ先頭

      ©2009-2025 Movatter.jp