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: iter: relax Pull to not panic because of thread locking state #67694

Open
eliasnaur opened this issue May 29, 2024 · 6 comments
Open
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Proposal
Milestone

Comments

@eliasnaur
Copy link
Contributor

Proposal Details

iter.Pull snapshots the thread lock count and locked thread, if any. Calling next, stop or yield with a different snapshot panics. This proposal is about lifting that restriction.

Adapted from my longer explanation at #65889 (comment), the proposal is to replace the panic behaviour with cooperative scheduling:

Let the locked thread (if any) and its lock count follow goroutine switches between caller and iterator, making next, stop and yield act as cooperative scheduling points. That is, the thread (and lock count) entering next, stop or yield is the same thread (and lock count) that resumes execution from the return of its counterpart. For example, calling next from (locked) thread T will see yield return (locked) on thread T and vice versa.

Notable consequences:

  • Deadlock free: no more than one goroutine has a particular thread locked at any one time.
  • A locked thread follows execution similar to rangefuncs: once LockOSThread is called by either caller or iterator, the execution trace has that thread locked from then on.
  • Thread "stealing" is possible. E.g. if the caller calls LockOSThread and the iterator yields from another goroutine. This is perhaps surprising, yet is consistent with rangefuncs.

Other than making iter.Pull more widely useful by eliminating panics, this proposal elegantly fixes #64755 and #64777 by thread stealing. For example:

// Package mainthreadapi implements a platform API that require the main thread.
package mainthreadapi

import "iter"
import "C"

func init() {
  // Lock the main thread.
  runtime.LockOSThread()
  next, _ := iter.Pull(func(yield func(struct{}) bool) {
       go yield(struct{}{}) // Pass a different thread to the caller.
       C.native_event_loop_that_require_the_main_thread_and_never_returns()
  })
  next() // Pass the main thread to the iterator
  // Continues on a different thread.
}

// API functions follow.
@hajimehoshi
Copy link
Member

hajimehoshi commented May 29, 2024

Just out of curiosity, will the next init be called on the main thread, or a different thread? If init is called on a different thread, wouldn't this break some existing libraries that assume init must be called on the main thread?

@eliasnaur
Copy link
Contributor Author

The next init will run on a different thread. I assume few packages require the main thread during initialization, and packages that do should add a check for the main thread.

Some packages call LockOSThread during init to lock the main thread to the main goroutine, but they wouldn't notice the thread is different. The package API meant to be called from the main goroutine need a main thread check anyway to guard against the user calling the package from a different goroutine.

@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals May 29, 2024
@ianlancetaylor ianlancetaylor added the compiler/runtime Issues related to the Go compiler and/or runtime. label May 29, 2024
@hajimehoshi
Copy link
Member

hajimehoshi commented May 30, 2024

I thought some OS APIs must be called from the main thread (= the thread that is for init/main), but I might misunderstand. I'll look for an example.

Another thing I came up with is the opposite pattern:

  1. Call runtime.LockOSThread in a library A
  2. Call iter.Pull in a library B
  3. The main function runs on a thread that is different from the locked thread in A

For example, if an OpenGL context is initialized for the current thread at 1, and the main function calls GL functions, the application no longer works due to 2.

@eliasnaur
Copy link
Contributor Author

eliasnaur commented May 30, 2024

I thought some OS APIs must be called from the main thread (= the thread that is for init/main), but I might misunderstand. I'll look for an example.

Absolutely, and in your example the "next init" that expect the main thread would fail, either immediately because of an is-main-thread check or because calls to its main-thread native API fails.

My point is that it's already the case the two or more packages all requiring the main thread needs some kind of coordination to work together. The iter.Pull pattern in my proposal merely allows a package a way to hide its main thread requirement in its public API.

Another thing I came up with is the opposite pattern:

1. Call `runtime.LockOSThread` in a library A

2. Call `iter.Pull` in a library B

3. The main function runs on a thread that is different from the locked thread in A

I consider it unlikely, or at least bad design, to actually use the main thread during initialization. LockOSThread during initialization is only to force the Go runtime to lock the main thread to the main function/goroutine.

For example, if an OpenGL context is initialized for the current thread at 1, and the main function calls GL functions, the application no longer works due to 2.

In your stated example, yes. However, a well-designed OpenGL package would not call LockOSThread until creation of an OpenGL context, because OpenGL only require some locked thread, not necessarily the main thread.

@thediveo
Copy link

thediveo commented May 30, 2024

locking the main thread in init is also needed to avoid tripping Linux kernel namespace leak checks, when the initial thread get only later locked and then tainted. Without initial locking the initial thread cannot be destroyed, but permanently wedged, to use Go's runtime terminology. Locking the initial thread ensures that when later needing locking for namespace switches will happen on non-initial go routines and this on threads than can be thrown away, the reason being that this happens, for instance as part of an http handler goroutine.

@mknyszek
Copy link
Contributor

mknyszek commented Jun 5, 2024

What makes me personally uncomfortable with the semantics you suggest is the goroutine executing init, without ever calling runtime.UnlockOSThread, has managed to lose its OS thread lock. This is very unintuitive and surprising, IMO.

I also don't see how that's preferable to something like RunOnMainThread (#64777). If multiple packages try to call something like C.native_event_loop_that_require_the_main_thread_and_never_returns() in this proposal, then I assume that the API call will fail (or hang, or crash in some weird way due to a race, or whatever it does when it's not actually called from the main thread). In the RunOnMainThread proposal (at least, without any additional tweaks), all but one of these packages' init functions will hang, but you don't actually violate any invariants.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Proposal
Projects
Status: No status
Status: Incoming
Development

No branches or pull requests

6 participants