Skip to content

Commit

Permalink
internal/core/adt: don't signal cleared schedulers
Browse files Browse the repository at this point in the history
A nodeContext is freed once its refCount reaches 0, which also means
that its scheduler gets cleared to be reused at a later time.

However, when extracting the jsonschema in issue3351.txtar with evalv3
and evaluating the resulting CUE, we would panic as the evaluator
would try to signal a scheduler which had been cleared as explained:

    --- FAIL: TestDecode/v3/decode-v3/issue3351 (0.00s)
        panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        [signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x722aec]

        goroutine 49 [running]:
        [...]
        panic({0xb0e260?, 0x11213e0?})
        	runtime/panic.go:785 +0x132
        cuelang.org/go/internal/core/adt.stateCompletions(0xc0003cd3d8)
        	./internal/core/adt/states.go:242 +0x2c
        cuelang.org/go/internal/core/adt.(*scheduler).signal(0xc0003cd3d8, 0x2b04?)
        	./internal/core/adt/sched.go:525 +0x3b
        cuelang.org/go/internal/core/adt.(*scheduler).process(0xc0002d07d8, 0x7eff, 0x4)
        	./internal/core/adt/sched.go:442 +0x3ee
        [...]

The reason was that a task was pointing to the cleared scheduler
via blockedOn. Given that it doesn't make sense for a task to be blocked
on a scheduler which has been entirely released and no longer used,
teach scheduler.clear to mark any tasks it blocks as no longer blocked.

TestDecode now tests all txtar files with the new evaluator too;
note that we didn't do that before this commit as issue3351.txtar
would panic, which is a fatal error that cuetxtar does not recover from.

It's not entirely clear that this is the perfect solution long term,
as perhaps there is another underlying bug, but for now this fix
seems to resolve the issue without any regressions, and is reasonable.

Fixes #3351.

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: I32ae93bfb4d4542d5cf4becf077c1e849f6584a5
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1198736
Reviewed-by: Roger Peppe <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Marcel van Lohuizen <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
  • Loading branch information
mvdan committed Aug 1, 2024
1 parent 95c818c commit 0d4258e
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 deletions.
6 changes: 4 additions & 2 deletions encoding/jsonschema/decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"cuelang.org/go/encoding/jsonschema"
"cuelang.org/go/encoding/yaml"
"cuelang.org/go/internal/astinternal"
"cuelang.org/go/internal/cuetdtest"
"cuelang.org/go/internal/cuetxtar"
_ "cuelang.org/go/pkg"
)
Expand All @@ -41,8 +42,9 @@ import (
// Set CUE_UPDATE=1 to update test files with the corresponding output.
func TestDecode(t *testing.T) {
test := cuetxtar.TxTarTest{
Root: "./testdata",
Name: "decode",
Root: "./testdata",
Name: "decode",
Matrix: cuetdtest.FullMatrix,
}
test.Run(t, func(t *cuetxtar.Test) {
cfg := &jsonschema.Config{}
Expand Down
11 changes: 11 additions & 0 deletions internal/core/adt/sched.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,17 @@ type scheduler struct {
func (s *scheduler) clear() {
// TODO(perf): free tasks into task pool

// Any tasks blocked on this scheduler are unblocked once the scheduler is cleared.
// Otherwise they might signal a cleared scheduler, which can panic.
//
// TODO(mvdan,mpvl): In principle, all blocks should have been removed when a scheduler
// is cleared. Perhaps this can happen when the scheduler is stopped prematurely.
// For now, this solution seems to work OK.
for _, t := range s.blocking {
t.blockedOn = nil
t.blockCondition = neverKnown
}

*s = scheduler{
ctx: s.ctx,
tasks: s.tasks[:0],
Expand Down

0 comments on commit 0d4258e

Please sign in to comment.