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

runtime: main_init_done can be implemented more efficiently #15943

Open
ianlancetaylor opened this issue Jun 2, 2016 · 7 comments
Open

runtime: main_init_done can be implemented more efficiently #15943

ianlancetaylor opened this issue Jun 2, 2016 · 7 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. Performance
Milestone
Unplanned

Comments

@ianlancetaylor
Copy link
Contributor

A C thread in a -buildmode={c-archive,c-shared} may call back into Go code before Go initialization is complete, in which case it must wait. It currently waits using a channel called main_init_done. In the review of https://golang.org/cl/23610 Dmitry points out that this can be done more efficiently. The callback will necessarily go through runtime·needm. We can arrange for needm to wait for initialization to be complete. It already waits for a new M to be created.

In fact, needm already has a race condition. It crashes if cgoHasExtraM is not true. That global variable is set to true early in Go initialization, but that initialization will be running in a separate thread. If a C thread is fast enough in calling Go, the program will crash because Go initialization has not yet reached the required point. We should avoid crashing if islibrary || isarchive; we should simply wait.
@ianlancetaylor ianlancetaylor added this to the Go1.8 milestone Jun 2, 2016
@ianlancetaylor ianlancetaylor self-assigned this Jun 2, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 11, 2016
@rsc rsc modified the milestones: Go1.9, Go1.8 Nov 11, 2016
@aclements
Copy link
Member

Ping @ianlancetaylor. Should this bump to Go1.10?

@ianlancetaylor
Copy link
Contributor Author

Bumping. Sigh.

@ianlancetaylor ianlancetaylor modified the milestones: Go1.10, Go1.9 Jun 8, 2017
@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017
@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Go1.12 Jul 10, 2018
@aclements aclements modified the milestones: Go1.12, Go1.13 Jan 8, 2019
@andybons andybons removed this from the Go1.13 milestone Jul 8, 2019
@bkgs
Copy link

bkgs commented Oct 11, 2020

Hi, I just bumped into this... are there possibly any updates/workarounds for this?

@ianlancetaylor
Copy link
Contributor Author

@bkgs What did you run into, exactly? This is mainly a performance issue. There is a theoretical bug but I don't know if it can actually occur in practice.

@bkgs
Copy link

bkgs commented Oct 14, 2020

@bkgs What did you run into, exactly? This is mainly a performance issue. There is a theoretical bug but I don't know if it can actually occur in practice.

@ianlancetaylor apologies for the lousy wording: I saw #15943 mentioned in a comment while clearing some assumptions on cgo. My question relates to the race condition in needm.

I was curious if there is a known tweak to rule this out entirely (e.g., would waiting for a Go-C call from a custom Go package init() before making C-Go calls make a difference beyond buying cgoHasExtraM init a bit more time?). It's not a big concern, we can trigger a C-Go call during startup and retry in the unlikely case of a crash.

@ianlancetaylor
Copy link
Contributor Author

Yes, I think you can avoid the potential bug entirely by waiting for a Go -> C call, perhaps from an init function, before making any C -> Go calls.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
@prattmic prattmic added this to the Unplanned milestone Jul 13, 2022
@gopherbot
Copy link

Change https://go.dev/cl/538376 mentions this issue: runtime: implement main_init_done more efficiently

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. Performance
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
8 participants
@rsc @quentinmit @andybons @prattmic @aclements @ianlancetaylor @gopherbot @bkgs