fixed unhandled panic and wrapped all internal goroutines
This commit is contained in:
@@ -7,6 +7,9 @@
|
||||
|
||||
- Fixed View collection `*` validator and added more friendly error messages ([#7761](https://github.com/pocketbase/pocketbase/issues/7761)).
|
||||
|
||||
- ⚠️ Security fix for unhandled panic in internal worker goroutines ([#7762](https://github.com/pocketbase/pocketbase/discussions/7762)).
|
||||
_To prevent this from showing again, all existing internal worker functions were wrapped with [`routine.SafeWrap(f)`](https://pkg.go.dev/github.com/pocketbase/pocketbase/tools/routine#SafeWrap) (auto recovers and returns any eventual panic as regular error)._
|
||||
|
||||
|
||||
## v0.39.6
|
||||
|
||||
|
||||
+3
-2
@@ -18,6 +18,7 @@ import (
|
||||
"github.com/pocketbase/pocketbase/core"
|
||||
"github.com/pocketbase/pocketbase/tools/filesystem"
|
||||
"github.com/pocketbase/pocketbase/tools/router"
|
||||
"github.com/pocketbase/pocketbase/tools/routine"
|
||||
"github.com/pocketbase/pocketbase/tools/types"
|
||||
"github.com/spf13/cast"
|
||||
)
|
||||
@@ -195,7 +196,7 @@ func (p *batchProcessor) Process(batch []*core.InternalRequest, timeout time.Dur
|
||||
p.stopCh <- struct{}{}
|
||||
}()
|
||||
|
||||
go func() {
|
||||
routine.FireAndForget(func() {
|
||||
err := p.process(txApp, batch, 0)
|
||||
|
||||
if err != nil {
|
||||
@@ -216,7 +217,7 @@ func (p *batchProcessor) Process(batch []*core.InternalRequest, timeout time.Dur
|
||||
}
|
||||
|
||||
p.errCh <- err
|
||||
}()
|
||||
})
|
||||
|
||||
select {
|
||||
case responseErr := <-p.errCh:
|
||||
|
||||
+13
-13
@@ -10,8 +10,8 @@ import (
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
validation "github.com/pocketbase/ozzo-validation/v4"
|
||||
"github.com/pocketbase/dbx"
|
||||
validation "github.com/pocketbase/ozzo-validation/v4"
|
||||
"github.com/pocketbase/pocketbase/core"
|
||||
"github.com/pocketbase/pocketbase/tools/hook"
|
||||
"github.com/pocketbase/pocketbase/tools/picker"
|
||||
@@ -260,7 +260,7 @@ func realtimeUpdateClientsAuth(app core.App, authRecord *core.Record) error {
|
||||
group := new(errgroup.Group)
|
||||
|
||||
for _, chunk := range chunks {
|
||||
group.Go(func() error {
|
||||
group.Go(routine.SafeWrap(func() error {
|
||||
for _, client := range chunk {
|
||||
clientAuth, _ := client.Get(RealtimeClientAuthKey).(*core.Record)
|
||||
if clientAuth != nil &&
|
||||
@@ -275,7 +275,7 @@ func realtimeUpdateClientsAuth(app core.App, authRecord *core.Record) error {
|
||||
}
|
||||
|
||||
return nil
|
||||
})
|
||||
}))
|
||||
}
|
||||
|
||||
return group.Wait()
|
||||
@@ -288,7 +288,7 @@ func realtimeUnsetClientsAuthByRecordModelOrProxy(app core.App, authModel core.M
|
||||
group := new(errgroup.Group)
|
||||
|
||||
for _, chunk := range chunks {
|
||||
group.Go(func() error {
|
||||
group.Go(routine.SafeWrap(func() error {
|
||||
for _, client := range chunk {
|
||||
clientAuth, _ := client.Get(RealtimeClientAuthKey).(*core.Record)
|
||||
if clientAuth != nil &&
|
||||
@@ -299,7 +299,7 @@ func realtimeUnsetClientsAuthByRecordModelOrProxy(app core.App, authModel core.M
|
||||
}
|
||||
|
||||
return nil
|
||||
})
|
||||
}))
|
||||
}
|
||||
|
||||
return group.Wait()
|
||||
@@ -312,7 +312,7 @@ func realtimeUnsetClientsAuthByCollection(app core.App, collection *core.Collect
|
||||
group := new(errgroup.Group)
|
||||
|
||||
for _, chunk := range chunks {
|
||||
group.Go(func() error {
|
||||
group.Go(routine.SafeWrap(func() error {
|
||||
for _, client := range chunk {
|
||||
clientAuth, _ := client.Get(RealtimeClientAuthKey).(*core.Record)
|
||||
if clientAuth != nil && clientAuth.Collection().Name == collection.Name {
|
||||
@@ -321,7 +321,7 @@ func realtimeUnsetClientsAuthByCollection(app core.App, collection *core.Collect
|
||||
}
|
||||
|
||||
return nil
|
||||
})
|
||||
}))
|
||||
}
|
||||
|
||||
return group.Wait()
|
||||
@@ -623,7 +623,7 @@ func realtimeBroadcastRecord(app core.App, action string, record *core.Record, d
|
||||
}
|
||||
|
||||
for _, chunk := range chunks {
|
||||
group.Go(func() error {
|
||||
group.Go(routine.SafeWrap(func() error {
|
||||
var clientAuth *core.Record
|
||||
|
||||
for _, client := range chunk {
|
||||
@@ -766,7 +766,7 @@ func realtimeBroadcastRecord(app core.App, action string, record *core.Record, d
|
||||
}
|
||||
|
||||
return nil
|
||||
})
|
||||
}))
|
||||
}
|
||||
|
||||
return group.Wait()
|
||||
@@ -782,7 +782,7 @@ func realtimeBroadcastDryCacheKey(app core.App, key string) error {
|
||||
group := new(errgroup.Group)
|
||||
|
||||
for _, chunk := range chunks {
|
||||
group.Go(func() error {
|
||||
group.Go(routine.SafeWrap(func() error {
|
||||
for _, client := range chunk {
|
||||
messages, ok := client.Get(key).([]subscriptions.Message)
|
||||
if !ok {
|
||||
@@ -801,7 +801,7 @@ func realtimeBroadcastDryCacheKey(app core.App, key string) error {
|
||||
}
|
||||
|
||||
return nil
|
||||
})
|
||||
}))
|
||||
}
|
||||
|
||||
return group.Wait()
|
||||
@@ -817,7 +817,7 @@ func realtimeUnsetDryCacheKey(app core.App, key string) error {
|
||||
group := new(errgroup.Group)
|
||||
|
||||
for _, chunk := range chunks {
|
||||
group.Go(func() error {
|
||||
group.Go(routine.SafeWrap(func() error {
|
||||
for _, client := range chunk {
|
||||
if client.Get(key) != nil {
|
||||
client.Unset(key)
|
||||
@@ -825,7 +825,7 @@ func realtimeUnsetDryCacheKey(app core.App, key string) error {
|
||||
}
|
||||
|
||||
return nil
|
||||
})
|
||||
}))
|
||||
}
|
||||
|
||||
return group.Wait()
|
||||
|
||||
+2
-2
@@ -1458,7 +1458,7 @@ func (app *BaseApp) initLogger() error {
|
||||
},
|
||||
})
|
||||
|
||||
go func() {
|
||||
routine.FireAndForget(func() {
|
||||
ctx := context.Background()
|
||||
|
||||
for {
|
||||
@@ -1469,7 +1469,7 @@ func (app *BaseApp) initLogger() error {
|
||||
handler.WriteAll(ctx)
|
||||
}
|
||||
}
|
||||
}()
|
||||
})
|
||||
|
||||
app.logger = slog.New(handler)
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"github.com/fatih/color"
|
||||
"github.com/fsnotify/fsnotify"
|
||||
"github.com/pocketbase/pocketbase/tools/hook"
|
||||
"github.com/pocketbase/pocketbase/tools/routine"
|
||||
"github.com/pocketbase/pocketbase/tools/security"
|
||||
)
|
||||
|
||||
@@ -152,7 +153,7 @@ func createNotifyDirWatcher(app App, instanceId string, localNotifyDirPath strin
|
||||
}
|
||||
|
||||
// watch
|
||||
go func() {
|
||||
routine.FireAndForget(func() {
|
||||
defer stopDebounceTimer()
|
||||
|
||||
for {
|
||||
@@ -204,7 +205,7 @@ func createNotifyDirWatcher(app App, instanceId string, localNotifyDirPath strin
|
||||
}
|
||||
}
|
||||
}
|
||||
}()
|
||||
})
|
||||
|
||||
return watcher, err
|
||||
}
|
||||
|
||||
@@ -33,6 +33,7 @@ import (
|
||||
"github.com/fsnotify/fsnotify"
|
||||
"github.com/pocketbase/pocketbase/core"
|
||||
"github.com/pocketbase/pocketbase/plugins/jsvm/internal/types/generated"
|
||||
"github.com/pocketbase/pocketbase/tools/routine"
|
||||
"github.com/pocketbase/pocketbase/tools/template"
|
||||
)
|
||||
|
||||
@@ -407,7 +408,7 @@ func (p *plugin) watchHooks() error {
|
||||
})
|
||||
|
||||
// start listening for events.
|
||||
go func() {
|
||||
routine.FireAndForget(func() {
|
||||
defer stopDebounceTimer()
|
||||
|
||||
for {
|
||||
@@ -437,7 +438,7 @@ func (p *plugin) watchHooks() error {
|
||||
color.Red("Watch error:", err)
|
||||
}
|
||||
}
|
||||
}()
|
||||
})
|
||||
|
||||
// add directories to watch
|
||||
//
|
||||
|
||||
+4
-4
@@ -186,21 +186,21 @@ func (pb *PocketBase) Execute() error {
|
||||
done := make(chan bool, 1)
|
||||
|
||||
// listen for interrupt signal to gracefully shutdown the application
|
||||
go func() {
|
||||
routine.FireAndForget(func() {
|
||||
sigch := make(chan os.Signal, 1)
|
||||
signal.Notify(sigch, os.Interrupt, syscall.SIGTERM)
|
||||
<-sigch
|
||||
|
||||
done <- true
|
||||
}()
|
||||
})
|
||||
|
||||
// execute the root command
|
||||
go func() {
|
||||
routine.FireAndForget(func() {
|
||||
// note: leave to the commands to decide whether to print their error
|
||||
pb.RootCmd.Execute()
|
||||
|
||||
done <- true
|
||||
}()
|
||||
})
|
||||
|
||||
<-done
|
||||
|
||||
|
||||
+2
-2
@@ -193,7 +193,7 @@ func (c *Cron) Start() {
|
||||
c.runDue(time.Now())
|
||||
|
||||
// run after each tick
|
||||
go func() {
|
||||
routine.FireAndForget(func() {
|
||||
for {
|
||||
select {
|
||||
case <-c.tickerDone:
|
||||
@@ -202,7 +202,7 @@ func (c *Cron) Start() {
|
||||
c.runDue(t)
|
||||
}
|
||||
}
|
||||
}()
|
||||
})
|
||||
})
|
||||
c.mux.Unlock()
|
||||
}
|
||||
|
||||
@@ -14,6 +14,7 @@ import (
|
||||
"strings"
|
||||
"sync"
|
||||
|
||||
"github.com/pocketbase/pocketbase/tools/routine"
|
||||
"golang.org/x/sync/errgroup"
|
||||
)
|
||||
|
||||
@@ -339,7 +340,7 @@ func (u *Uploader) multipartUpload(ctx context.Context, initPart []byte, optReqF
|
||||
if len(initPart) != 0 {
|
||||
totalWorkers--
|
||||
initPartNumber := u.lastPartNumber
|
||||
g.Go(func() error {
|
||||
g.Go(routine.SafeWrap(func() error {
|
||||
mp, err := u.uploadPart(ctx, initPartNumber, initPart, optReqFuncs...)
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -350,13 +351,13 @@ func (u *Uploader) multipartUpload(ctx context.Context, initPart []byte, optReqF
|
||||
u.mu.Unlock()
|
||||
|
||||
return nil
|
||||
})
|
||||
}))
|
||||
}
|
||||
|
||||
totalWorkers = max(totalWorkers, 1)
|
||||
|
||||
for i := 0; i < totalWorkers; i++ {
|
||||
g.Go(func() error {
|
||||
g.Go(routine.SafeWrap(func() error {
|
||||
for {
|
||||
part, num, err := u.nextPart()
|
||||
if err != nil {
|
||||
@@ -377,7 +378,7 @@ func (u *Uploader) multipartUpload(ctx context.Context, initPart []byte, optReqF
|
||||
}
|
||||
|
||||
return nil
|
||||
})
|
||||
}))
|
||||
}
|
||||
|
||||
return g.Wait()
|
||||
|
||||
@@ -42,6 +42,7 @@ import (
|
||||
|
||||
"github.com/pocketbase/pocketbase/tools/filesystem/blob"
|
||||
"github.com/pocketbase/pocketbase/tools/filesystem/internal/s3blob/s3"
|
||||
"github.com/pocketbase/pocketbase/tools/routine"
|
||||
)
|
||||
|
||||
const defaultPageSize = 1000
|
||||
@@ -359,7 +360,7 @@ func (w *writer) Write(p []byte) (int, error) {
|
||||
// error uploading to S3.
|
||||
func (w *writer) open(r io.Reader, closePipeOnError bool) {
|
||||
// This goroutine will keep running until Close, unless there's an error.
|
||||
go func() {
|
||||
routine.FireAndForget(func() {
|
||||
defer func() {
|
||||
close(w.donec)
|
||||
}()
|
||||
@@ -378,7 +379,7 @@ func (w *writer) open(r io.Reader, closePipeOnError bool) {
|
||||
}
|
||||
w.err = err
|
||||
}
|
||||
}()
|
||||
})
|
||||
}
|
||||
|
||||
// Close completes the writer and closes it. Any error occurring during write
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package routine
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"log"
|
||||
"runtime"
|
||||
"sync"
|
||||
@@ -33,3 +34,17 @@ func FireAndForget(f func(), wg ...*sync.WaitGroup) {
|
||||
f()
|
||||
}()
|
||||
}
|
||||
|
||||
// SafeWrap wraps the provided function with auto panic recover handling
|
||||
// and returns any eventual panic as regular error.
|
||||
func SafeWrap(f func() error) func() error {
|
||||
return func() (err error) {
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
err = fmt.Errorf("[SafeWrap] recovered from panic: %v", r)
|
||||
}
|
||||
}()
|
||||
|
||||
return f()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package routine_test
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
@@ -12,7 +13,7 @@ func TestFireAndForget(t *testing.T) {
|
||||
|
||||
fn := func() {
|
||||
called = true
|
||||
panic("test")
|
||||
panic("test_recover")
|
||||
}
|
||||
|
||||
wg := &sync.WaitGroup{}
|
||||
@@ -22,6 +23,29 @@ func TestFireAndForget(t *testing.T) {
|
||||
wg.Wait()
|
||||
|
||||
if !called {
|
||||
t.Error("Expected fn to be called.")
|
||||
t.Fatal("Expected fn to be called.")
|
||||
}
|
||||
}
|
||||
|
||||
func TestSafeWrap(t *testing.T) {
|
||||
called := false
|
||||
|
||||
fn := func() error {
|
||||
called = true
|
||||
panic("test_recover")
|
||||
}
|
||||
|
||||
err := routine.SafeWrap(fn)()
|
||||
|
||||
if !called {
|
||||
t.Fatal("Expected fn to be called.")
|
||||
}
|
||||
|
||||
if err == nil {
|
||||
t.Fatal("Expected fn panic to be converted to error")
|
||||
}
|
||||
|
||||
if !strings.Contains(err.Error(), "test_recover") {
|
||||
t.Fatal("Expected the returned error to contain the recovered panic value")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"github.com/pocketbase/dbx"
|
||||
"github.com/pocketbase/pocketbase/tools/dbutils"
|
||||
"github.com/pocketbase/pocketbase/tools/inflector"
|
||||
"github.com/pocketbase/pocketbase/tools/routine"
|
||||
"golang.org/x/sync/errgroup"
|
||||
)
|
||||
|
||||
@@ -336,8 +337,8 @@ func (s *Provider) Exec(items any) (*Result, error) {
|
||||
if !s.skipTotal {
|
||||
// execute the 2 queries concurrently
|
||||
errg := new(errgroup.Group)
|
||||
errg.Go(countExec)
|
||||
errg.Go(modelsExec)
|
||||
errg.Go(routine.SafeWrap(countExec))
|
||||
errg.Go(routine.SafeWrap(modelsExec))
|
||||
if err := errg.Wait(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user