Skip to content

Commit

Permalink
playground: prevent caching memory-related runtime errors
Browse files Browse the repository at this point in the history
The existing implementation only prevents code caching if the code has
certain compile-time errors. Code with deterministic runtime errors, such as
out of memory errors, are being cached. This causes errors to be
returned in the Go Tour if a different user's run caused an error.
The cache had to be manually cleared in order to clean out the bad run.
This change prevents code that threw an "out of memory" or a "cannot
allocate memory" error from being cached.

Fixes golang/go#26926

Change-Id: Ib281f3076bc674e7c2f08bf9b5c4be36da22d28e
Reviewed-on: https://go-review.googlesource.com/130035
Reviewed-by: Andrew Bonventre <[email protected]>
  • Loading branch information
katiehockman committed Aug 21, 2018
1 parent 0757b4e commit e07747d
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 0 deletions.
15 changes: 15 additions & 0 deletions sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ const (
progName = "prog.go"
)

// Runtime error strings that will not be cached if found in an Event message.
var nonCachingRuntimeErrors = []string{"out of memory", "cannot allocate memory"}

type request struct {
Body string
}
Expand Down Expand Up @@ -87,6 +90,18 @@ func (s *server) commandHandler(cachePrefix string, cmdFunc func(*request) (*res
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
return
}
for _, el := range resp.Events {
if el.Kind != "stderr" {
continue
}
for _, e := range nonCachingRuntimeErrors {
if strings.Contains(el.Message, e) {
s.log.Errorf("cmdFunc runtime error: %q", el.Message)
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
return
}
}
}
if err := s.cache.Set(key, resp); err != nil {
s.log.Errorf("cache.Set(%q, resp): %v", key, err)
}
Expand Down
13 changes: 13 additions & 0 deletions server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,15 @@ func TestCommandHandler(t *testing.T) {
if r.Body == "error" {
return &response{Errors: "errors"}, nil
}
if r.Body == "oom-error" {
// To throw an oom in a local playground instance, increase the server timeout
// to 20 seconds (within sandbox.go), spin up the Docker instance and run
// this code: https://play.golang.org/p/aaCv86m0P14.
return &response{Events: []Event{{"out of memory", "stderr", 0}}}, nil
}
if r.Body == "allocate-memory-error" {
return &response{Events: []Event{{"cannot allocate memory", "stderr", 0}}}, nil
}
resp := &response{Events: []Event{{r.Body, "stdout", 0}}}
return resp, nil
})
Expand All @@ -191,6 +200,10 @@ func TestCommandHandler(t *testing.T) {
[]byte(`{"Errors":"errors","Events":null}
`),
},
{"Out of memory error in response body event message", http.MethodPost, http.StatusInternalServerError,
[]byte(`{"Body":"oom-error"}`), nil},
{"Cannot allocate memory error in response body event message", http.MethodPost, http.StatusInternalServerError,
[]byte(`{"Body":"allocate-memory-error"}`), nil},
}

for _, tc := range testCases {
Expand Down

0 comments on commit e07747d

Please sign in to comment.