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

proposal: sync: add blocking methods with Context parameter to Once and WaitGroup #25312

Closed
daviddengcn opened this issue May 9, 2018 · 12 comments

Comments

@daviddengcn
Copy link

daviddengcn commented May 9, 2018

Please answer these questions before submitting your issue. Thanks!

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

1.10.2

Does this issue reproduce with the latest release?

n/a

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

all

What did you do?

n/a

What did you expect to see?

sync.Once once;
once.DoWithContext(ctx, f) // when ctx is cancelled, f is still running but the caller can return immediately

Similar to WaitGroup.WaitWithContext.

What did you see instead?

n/a

@ianlancetaylor ianlancetaylor changed the title Add Context parameter to blocking methods of sync.Once and sync.WaitGroup proposal: sync: add blocking methods with Context parameter to Once and WaitGroup May 9, 2018
@gopherbot gopherbot added this to the Proposal milestone May 9, 2018
@daviddengcn
Copy link
Author

daviddengcn commented May 9, 2018

Use case:
sync.Once is used to make a heavy initiailization but the initialization is done lazily, i.e. it's called the first time the resource is needed. When the request is canceled, the initialization should go ahead but the handler expect to return and do related cleanup immediately.

@ianlancetaylor
Copy link
Contributor

I don't see how this could work in general. If you call once.DoWithContext you don't know whether the value has been initialized or not.

Perhaps something like this code would do what you want:

    c := make(chan bool, 1)
    go func() {
        once.Do(f)
        c <- true
    }()
    select {
    case <-c:
    case <-ctxt.Done():
        return
    }

Here it is clear that the context is being handled by this code, and doesn't really have anything to do with once.

@daviddengcn
Copy link
Author

daviddengcn commented May 9, 2018

DoWithContext can return an error

if err := once.DoWithContext(ctx, connect_to_server); err != nil {
  return err
}
// Connection created.

@daviddengcn
Copy link
Author

daviddengcn commented May 9, 2018

@ianlancetaylor, the code fragment you provided is my current workaround but implemented within sync.Once, an atomic checking can be done before the goroutine is started and the channel is made.

@ianlancetaylor
Copy link
Contributor

Making channels and goroutines are cheap. My point is that for that use case the Context argument doesn't actually have anything to with the Once operation. We're very unlikely to add new API for this.

If the efficiency really matters here, which I'm skeptical of, then I think a cleaner approach would be to add a Initialized method to sync.Once to check whether it has already been initialized. But although I think that would be cleaner we are also very unlikely to add new API for that, since it invites misuse.

If efficiency really matters here for you, I suggest that you simply copy and modify the sync.Once code for your own purposes. It's less than 50 lines including a lengthy comment.

@daviddengcn
Copy link
Author

daviddengcn commented May 9, 2018

I believe it'll be less than 50 lines because once.go itself is only 46 lines. (why we provide sync.Once at all)

context is newly introduced (into Golang built-in lib) so I think majority of blocking methods should try to be context friendly, i.e. it returns when the caller's context is cancelled. When context is more and more widely used, this request is not a rare case for me.

@ianlancetaylor
Copy link
Contributor

Context is very useful for cases where an operation can be canceled or time out. In your proposal the operation can not be canceled.

@daviddengcn
Copy link
Author

In my case, the caller is in handling an operation that can be cancelled but may need lazily do an intialization op (which cannot be cancelled) once.

@ianlancetaylor
Copy link
Contributor

Yes, exactly: the initialization operation can not be canceled.

@rsc
Copy link
Contributor

rsc commented May 21, 2018

The initialization, once started, can't be cancelled, but if initialization were running in a different goroutine then waiting for the initialization could be cancelled. Same for waitgroup. So I guess it's possible, although DoWithContext is 6X longer than Do.

@rsc
Copy link
Contributor

rsc commented Jun 4, 2018

This seems like it is useful only in limited circumstances. And sync.Once is not making any special use of APIs internal to the standard library. So it seems like the right thing to do is to just make a new package that is customized to your use. Feel free to start with sync.Once (respecting the license of course).

@rsc rsc closed this as completed Jun 4, 2018
@virtuald
Copy link

virtuald commented Aug 7, 2018

I have a similar need as the OP. For future visitors to this issue, I've implemented my solution to this at https://github.com/virtuald/go-oncectx

@golang golang locked and limited conversation to collaborators Aug 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants