Skip to content

Commit

Permalink
Add a mutex to js.VU to prevent multiple asynchrnous calls to RunOnce
Browse files Browse the repository at this point in the history
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 :)
  • Loading branch information
mstoykov committed Nov 25, 2019
1 parent e3aa1aa commit a2e1c6f
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 0 deletions.
6 changes: 6 additions & 0 deletions js/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"net/http"
"net/http/cookiejar"
"strconv"
"sync"
"time"

"github.com/dop251/goja"
Expand Down Expand Up @@ -193,6 +194,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{}{
Expand Down Expand Up @@ -372,6 +374,8 @@ type VU struct {
// goroutine per call.
interruptTrackedCtx context.Context
interruptCancel context.CancelFunc

m *sync.Mutex
}

// Verify that VU implements lib.VU
Expand All @@ -385,6 +389,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())
Expand Down
51 changes: 51 additions & 0 deletions js/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"net/http"
"os"
"strings"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -534,6 +535,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";
Expand Down

0 comments on commit a2e1c6f

Please sign in to comment.