From e13f4b900d5ecc972112856d7ee497a54c39b8d5 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Mon, 25 Nov 2019 15:12:58 +0200 Subject: [PATCH] Add a mutex to js.VU to prevent multiple asynchrnous calls to RunOnce This should fix #867. This is not ... a great fix in the sense that while doing it I found out we are also asynchrnously touch VU.Runtime which should also not happen and the currently added mutex should probably be locked in most other functions, but this should fix the immediate problems and the other should be fixed in a more ... complete redesign :) --- js/runner.go | 6 ++++++ js/runner_test.go | 51 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/js/runner.go b/js/runner.go index 4d3f97635bc..9c14ea12ce5 100644 --- a/js/runner.go +++ b/js/runner.go @@ -28,6 +28,7 @@ import ( "net/http" "net/http/cookiejar" "strconv" + "sync" "time" "github.com/dop251/goja" @@ -187,6 +188,7 @@ func (r *Runner) newVU(samplesOut chan<- stats.SampleContainer) (*VU, error) { Console: r.console, BPool: bpool.NewBufferPool(100), Samples: samplesOut, + m: &sync.Mutex{}, } vu.Runtime.Set("console", common.Bind(vu.Runtime, vu.Console, vu.Context)) common.BindToGlobal(vu.Runtime, map[string]interface{}{ @@ -354,6 +356,8 @@ type VU struct { // goroutine per call. interruptTrackedCtx context.Context interruptCancel context.CancelFunc + + m *sync.Mutex } // Verify that VU implements lib.VU @@ -367,6 +371,8 @@ func (u *VU) Reconfigure(id int64) error { } func (u *VU) RunOnce(ctx context.Context) error { + u.m.Lock() + defer u.m.Unlock() // Track the context and interrupt JS execution if it's cancelled. if u.interruptTrackedCtx != ctx { interCtx, interCancel := context.WithCancel(context.Background()) diff --git a/js/runner_test.go b/js/runner_test.go index ddcb80fc2f9..e090b1474c3 100644 --- a/js/runner_test.go +++ b/js/runner_test.go @@ -33,6 +33,7 @@ import ( "net/http" "os" "strings" + "sync" "testing" "time" @@ -512,6 +513,56 @@ func TestVURunInterrupt(t *testing.T) { } } +func TestVURunInterruptDoesntPanic(t *testing.T) { + //TODO: figure out why interrupt sometimes fails... data race in goja? + if isWindows { + t.Skip() + } + + r1, err := getSimpleRunner("/script.js", ` + export default function() { while(true) {} } + `) + require.NoError(t, err) + require.NoError(t, r1.SetOptions(lib.Options{Throw: null.BoolFrom(true)})) + + r2, err := NewFromArchive(r1.MakeArchive(), lib.RuntimeOptions{}) + require.NoError(t, err) + testdata := map[string]*Runner{"Source": r1, "Archive": r2} + for name, r := range testdata { + name, r := name, r + t.Run(name, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Minute) + defer cancel() + samples := make(chan stats.SampleContainer, 100) + defer close(samples) + go func() { + for range samples { + } + }() + var wg sync.WaitGroup + + vu, err := r.newVU(samples) + require.NoError(t, err) + for i := 0; i < 1000; i++ { + wg.Add(1) + newCtx, newCancel := context.WithCancel(ctx) + var ch = make(chan struct{}) + go func() { + defer wg.Done() + close(ch) + vuErr := vu.RunOnce(newCtx) + assert.Error(t, vuErr) + assert.Contains(t, vuErr.Error(), "context cancelled") + }() + <-ch + time.Sleep(time.Millisecond * 1) // NOTE: increase this in case of problems ;) + newCancel() + } + wg.Wait() + }) + } +} + func TestVUIntegrationGroups(t *testing.T) { r1, err := getSimpleRunner("/script.js", ` import { group } from "k6";