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

Improve logic for gopherjs to assess in which thread it is running and enforce Run/ShowAndRun to be called from the main goroutine. #2780

Closed
Bluebugs opened this issue Feb 14, 2022 · 9 comments
Labels
blocker Items that would block a forthcoming release

Comments

@Bluebugs
Copy link
Contributor

Is your feature request related to a problem? Please describe:

This is a place to continue discussing about PR : #2752 which was accidentally merged.

Is it possible to construct a solution with the existing API?

This discussion should hopefully not lead to any new API.

Describe the solution you'd like to see:

Something better than what was landed.

@Bluebugs
Copy link
Contributor Author

For context, the main difference between gopherjs and all other platform so far is that it only run one thread (all the atomic are for example implemented as just direct access to the value). In this context, their is no difference between the main goroutine and any other goroutine as they will be executed with the same context.

The issue is most likely that gopherjs stacks are not being reported in the same way that go does.

@andydotxyz andydotxyz added the blocker Items that would block a forthcoming release label Feb 15, 2022
@andydotxyz
Copy link
Member

(adding this as a blocker to develop branch so we resolve the conversation before release)

@Bluebugs
Copy link
Contributor Author

There is actually no stack information available in gopherjs that would include any reference to a goroutine number or anything.

@andydotxyz
Copy link
Member

I think that @Jacalz and I don't understand how gopherjs works in this case.
Will code like go func() { a.Run() }() compile and run or not?
Initially it looked like we should fix goroutineID instead, but my understanding is limited so more info could help :).

@Bluebugs
Copy link
Contributor Author

Yes, that code will run. gopherjs does support all the go language. The difference is that it has no thread support nor can it interrupt the running code. Instead what it does is that at key go call, it will check what are the available goroutine that need to run and switch then. It means that atomic for example can safely become direct access to the underlying value. Or running in the main thread will always be technically true as there is only one thread.

As for the problem with goroutineID, there is no ID of any sort in the stack information that could be relied on.

@andydotxyz
Copy link
Member

As for the problem with goroutineID, there is no ID of any sort in the stack information that could be relied on.

Is it always ID 1 or 0?

@Bluebugs
Copy link
Contributor Author

There is just no ID. Neither 1 or 0, just none.

@andydotxyz
Copy link
Member

Oh of course, it pulls it from the stack trace I forgot sorry.
I wonder, should we let it default to 0 and have the lookup return 0 for gopherjs, that way the codepath through check remains in place?
What do you think @Jacalz, I think that or what is already in are the basic options...

@Jacalz
Copy link
Member

Jacalz commented Mar 6, 2022

Sorry for the late reply. Yes, I think you are right. That sounds like a sensible solution.

Bluebugs pushed a commit that referenced this issue Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker Items that would block a forthcoming release
Projects
None yet
Development

No branches or pull requests

3 participants