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

runtime: potential for self-deadlock in runtime write barriers #28993

Open
mknyszek opened this Issue Nov 28, 2018 · 1 comment

Comments

Projects
None yet
3 participants
@mknyszek
Contributor

mknyszek commented Nov 28, 2018

In the runtime there's an implicit invariant that's being maintained: write barriers may not be called into while the mheap.lock is held or the runtime could deadlock. This invariant is not documented anywhere nor is it enforced via nowritebarrier or nowritebarrierrec annotations.

Consider the following situation:

  1. Call into runtime.XXX
  2. runtime.XXX grabs the heap lock.
  3. runtime.XXX assigns a heap pointer value into some location.
  4. The write barrier is on and is invoked.
  5. The write barrier attempts to enqueue the pointer into a marking work buffer.
  6. There are no empty work buffers available to enqueue the pointer for marking.
  7. Call into mheap.allocManual to allocate space for more work buffers.
  8. mheap.allocManual attempts to grab the heap lock.
  9. Deadlock.

After some manual inspection, I'm fairly convinced that this isn't really a problem today because everywhere the heap lock is held, only notinheap-annotated structures are manipulated, so no write barriers are ever generated in those sections.

However, this invariant adds an additional mental burden when making changes to the runtime, and one day we may want to allow such write barriers (see https://go-review.googlesource.com/c/go/+/46751/12/src/runtime/mgc.go#266 for example), so perhaps the invariant doesn't need to exist at all.

traceBufs for example are managed via sysAlloc and sysFree directly, whereas GC mark work buffers live in heap-allocated spans. If we were to move GC mark work buffers to use the same model, this would effectively remove this invariant. Making this change will require some work, so I'm kicking this issue to 1.13.

CC @aclements @RLH

@gopherbot

This comment has been minimized.

gopherbot commented Nov 28, 2018

Change https://golang.org/cl/151540 mentions this issue: runtime: avoid holding the heap lock for gcPressureChange write barrier

@ALTree ALTree added this to the Go1.13 milestone Nov 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment