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

[Doc] Explain why goroutine sample is threadsafe #252

Open
xxgreg opened this issue Feb 14, 2023 · 4 comments
Open

[Doc] Explain why goroutine sample is threadsafe #252

xxgreg opened this issue Feb 14, 2023 · 4 comments
Labels
documentation Improvements or additions to documentation

Comments

@xxgreg
Copy link

xxgreg commented Feb 14, 2023

What are you really trying to do?

I'm learning how to use workflow.Go.

Reading the goroutine sample code I thought there was a race condition, as multiple concurrent goroutines are calling append on the same slice in the outer scope.

See: https://github.com/temporalio/samples-go/blob/main/goroutine/goroutine_workflow.go

i.e.

var results []string 
...
workflow.Go(ctx, func(gCtx workflow.Context) {
    ...
    results = append(results, result2)
})

It turns out that this isn't a race condition, as workflow.Go is different than a goroutine, so it's probably worth explaining this difference in the sample.

@xxgreg xxgreg added the bug Something isn't working label Feb 14, 2023
@xxgreg
Copy link
Author

xxgreg commented Feb 14, 2023

So according to this comment - this isn't a race condition.

https://community.temporal.io/t/is-workflow-go-safe-for-concurrency/6722

@Quinn-With-Two-Ns Quinn-With-Two-Ns added documentation Improvements or additions to documentation and removed bug Something isn't working labels Feb 14, 2023
@Quinn-With-Two-Ns
Copy link
Contributor

Yes this is thread safe as only one coroutine is ever run at a time. I see no harm in adding an extra comment explaining why

@Quinn-With-Two-Ns Quinn-With-Two-Ns changed the title [Bug] race condition in goroutine sample [Doc] Explain why goroutine sample is threadsafe Feb 14, 2023
@Quinn-With-Two-Ns
Copy link
Contributor

I updated the github issue to reflect the actual issue

@ioannist
Copy link

ioannist commented May 1, 2023

I was actually wondering the same thing. It seems that workflow "goroutines" are not really goroutines in the Go sense.

It should be more clear how workflow goroutines differ from actual goroutines.

I was also wondering what are the advantages/disadvantages of branch or splitmerge-selector vs. goroutine examples. They seem to be serving very similar use cases, very differently. It's probably worth it to cross-reference and compare the two in their respective descriptions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants