[pull] master from pocketbase:master

CHANGELOG.md

@@ -1,3 +1,13 @@
+## v0.40.4
+
+- Fixed migration deadlock if a logs db write happens to run while the migration is still executing ([#7836](https://github.com/pocketbase/pocketbase/issues/7836)).
+
+- `app.ResetBootstrapState()` was soft-deprecated in favour of `app.ClearBootstrap()`.
+    _Additionally a new `app.OnClearBootstrap()` hook was added to allow clearing custom allocated `OnBootstrap` resources in case the app uses a non-standard initialization (e.g. doesn't call `Start()` or intentionally skip the `OnTerminate` hook)._
+
+- Bumped `golang.org/x/*` dependencies.
+
+
 ## v0.40.3
 
 - Write the status header for JSON responses only if the fields picker succeed or has acceptable fallback.

core/app.go

@@ -54,11 +54,16 @@ type App interface {
 	// Bootstrap initializes the application
 	// (aka. create data dir, open db connections, load settings, etc.).
 	//
-	// It will call ResetBootstrapState() if the application was already bootstrapped.
+	// It calls ClearBootstrap() if the application was already bootstrapped.
 	Bootstrap() error
 
-	// ResetBootstrapState releases the initialized core app resources
+	// ClearBootstrap releases the initialized core app resources
 	// (closing db connections, stopping cron ticker, etc.).
+	//
+	// This method is no-op if the application is not bootstrapped yet.
+	ClearBootstrap() error
+
+	// Deprecated: use ClearBootstrap().
 	ResetBootstrapState() error
 
 	// DataDir returns the app data directory path.
@@ -713,6 +718,13 @@ type App interface {
 	// resources (db, app settings, etc).
 	OnBootstrap() *hook.Hook[*BootstrapEvent]
 
+	// OnBootstrapClear hook is triggered when clearing the main application
+	// resources (db connections, cron, logger, etc.)
+	//
+	// It is usually invoked automatically right before app termination
+	// or when manually calling app.ClearBootstrap().
+	OnBootstrapClear() *hook.Hook[*BootstrapEvent]
+
 	// OnServe hook is triggered when the app web server is started
 	// (after starting the TCP listener but before initializing the blocking serve task),
 	// allowing you to adjust its options and attach new routes or middlewares.

core/base.go

@@ -12,6 +12,7 @@ import (
 	"regexp"
 	"runtime"
 	"strings"
+	"sync/atomic"
 	"time"
 
 	"github.com/fatih/color"
@@ -89,11 +90,12 @@ type BaseApp struct {
 	auxNonconcurrentDB  dbx.Builder
 
 	// app event hooks
-	onBootstrap     *hook.Hook[*BootstrapEvent]
-	onServe         *hook.Hook[*ServeEvent]
-	onTerminate     *hook.Hook[*TerminateEvent]
-	onBackupCreate  *hook.Hook[*BackupEvent]
-	onBackupRestore *hook.Hook[*BackupEvent]
+	onBootstrap      *hook.Hook[*BootstrapEvent]
+	onBootstrapClear *hook.Hook[*BootstrapEvent]
+	onServe          *hook.Hook[*ServeEvent]
+	onTerminate      *hook.Hook[*TerminateEvent]
+	onBackupCreate   *hook.Hook[*BackupEvent]
+	onBackupRestore  *hook.Hook[*BackupEvent]
 
 	// db model hooks
 	onModelValidate           *hook.Hook[*ModelEvent]
@@ -249,6 +251,7 @@ func NewBaseApp(config BaseAppConfig) *BaseApp {
 func (app *BaseApp) initHooks() {
 	// app event hooks
 	app.onBootstrap = &hook.Hook[*BootstrapEvent]{}
+	app.onBootstrapClear = &hook.Hook[*BootstrapEvent]{}
 	app.onServe = &hook.Hook[*ServeEvent]{}
 	app.onTerminate = &hook.Hook[*TerminateEvent]{}
 	app.onBackupCreate = &hook.Hook[*BackupEvent]{}
@@ -405,14 +408,14 @@ func (app *BaseApp) IsBootstrapped() bool {
 // Bootstrap initializes the application
 // (aka. create data dir, open db connections, load settings, etc.).
 //
-// It will call ResetBootstrapState() if the application was already bootstrapped.
+// It calls ClearBootstrap() if the application was already bootstrapped.
 func (app *BaseApp) Bootstrap() error {
 	event := &BootstrapEvent{}
 	event.App = app
 
 	err := app.OnBootstrap().Trigger(event, func(e *BootstrapEvent) error {
-		// clear resources of previous core state (if any)
-		if err := app.ResetBootstrapState(); err != nil {
+		// clear previous bootstrap state (if any)
+		if err := app.ClearBootstrap(); err != nil {
 			return err
 		}
 
@@ -460,41 +463,55 @@ func (app *BaseApp) Bootstrap() error {
 	return err
 }
 
-type closer interface {
-	Close() error
+// Deprecated: use [ClearBootstrap].
+func (app *BaseApp) ResetBootstrapState() error {
+	return app.ClearBootstrap()
 }
 
-// ResetBootstrapState releases the initialized core app resources
+// ClearBootstrap releases the initialized core app resources
 // (closing db connections, stopping cron ticker, etc.).
-func (app *BaseApp) ResetBootstrapState() error {
-	app.Cron().Stop()
+//
+// This method is no-op if the application is not bootstrapped yet.
+func (app *BaseApp) ClearBootstrap() error {
+	if !app.IsBootstrapped() {
+		return nil
+	}
 
-	var errs []error
+	event := &BootstrapEvent{}
+	event.App = app
 
-	dbs := []*dbx.Builder{
-		&app.concurrentDB,
-		&app.nonconcurrentDB,
-		&app.auxConcurrentDB,
-		&app.auxNonconcurrentDB,
-	}
+	return app.OnBootstrapClear().Trigger(event, func(e *BootstrapEvent) error {
+		type closer interface {
+			Close() error
+		}
 
-	for _, db := range dbs {
-		if db == nil {
-			continue
+		var errs []error
+
+		dbs := []*dbx.Builder{
+			&app.concurrentDB,
+			&app.nonconcurrentDB,
+			&app.auxConcurrentDB,
+			&app.auxNonconcurrentDB,
 		}
-		if v, ok := (*db).(closer); ok {
-			if err := v.Close(); err != nil {
-				errs = append(errs, err)
+
+		for _, db := range dbs {
+			if db == nil {
+				continue
 			}
+			if v, ok := (*db).(closer); ok {
+				if err := v.Close(); err != nil {
+					errs = append(errs, err)
+				}
+			}
+			*db = nil
 		}
-		*db = nil
-	}
 
-	if len(errs) > 0 {
-		return errors.Join(errs...)
-	}
+		if len(errs) > 0 {
+			return errors.Join(errs...)
+		}
 
-	return nil
+		return nil
+	})
 }
 
 // DB returns the default app data.db builder instance.
@@ -817,7 +834,7 @@ func (app *BaseApp) Restart() error {
 	event.IsRestart = true
 
 	return app.OnTerminate().Trigger(event, func(e *TerminateEvent) error {
-		_ = e.App.ResetBootstrapState()
+		_ = e.App.ClearBootstrap()
 
 		// attempt to restart the bootstrap process in case execve returns an error for some reason
 		defer func() {
@@ -860,6 +877,10 @@ func (app *BaseApp) OnBootstrap() *hook.Hook[*BootstrapEvent] {
 	return app.onBootstrap
 }
 
+func (app *BaseApp) OnBootstrapClear() *hook.Hook[*BootstrapEvent] {
+	return app.onBootstrapClear
+}
+
 func (app *BaseApp) OnServe() *hook.Hook[*ServeEvent] {
 	return app.onServe
 }
@@ -1413,7 +1434,15 @@ func (app *BaseApp) registerBaseHooks() {
 		Id: "__pbCronStart__",
 		Func: func(e *ServeEvent) error {
 			app.Cron().Start()
+			return e.Next()
+		},
+		Priority: 999,
+	})
 
+	app.OnBootstrapClear().Bind(&hook.Handler[*BootstrapEvent]{
+		Id: "__pbCronStop__",
+		Func: func(e *BootstrapEvent) error {
+			app.Cron().Stop()
 			return e.Next()
 		},
 		Priority: 999,
@@ -1470,9 +1499,41 @@ func getLoggerMinLevel(app App) slog.Level {
 }
 
 func (app *BaseApp) initLogger() error {
+	var stopped atomic.Bool
+
 	duration := 3 * time.Second
 	ticker := time.NewTicker(duration)
-	done := make(chan bool, 1)
+
+	done := make(chan struct{}, 1)
+
+	runLogsWrite := func(logs []*logger.Log) {
+		if !app.IsBootstrapped() || app.Settings().Logs.MaxDays == 0 {
+			return
+		}
+
+		// write the accumulated logs
+		//
+		// note: based on several local tests there is no
+		// significant performance difference between small number
+		// of separate write queries vs 1 big INSERT
+		app.AuxRunInTransaction(func(txApp App) error {
+			model := &Log{}
+			for _, l := range logs {
+				model.MarkAsNew()
+				model.Id = GenerateDefaultRandomId()
+				model.Level = int(l.Level)
+				model.Message = l.Message
+				model.Data = l.Data
+				model.Created, _ = types.ParseDateTime(l.Time)
+
+				if err := txApp.AuxSave(model); err != nil {
+					log.Println("Failed to write log", model, err)
+				}
+			}
+
+			return nil
+		})
+	}
 
 	handler := logger.NewBatchHandler(logger.BatchOptions{
 		Level:     getLoggerMinLevel(app),
@@ -1487,35 +1548,25 @@ func (app *BaseApp) initLogger() error {
 				}
 			}
 
-			ticker.Reset(duration)
+			if !stopped.Load() {
+				ticker.Reset(duration)
+			}
 
 			return app.Settings().Logs.MaxDays > 0
 		},
 		WriteFunc: func(ctx context.Context, logs []*logger.Log) error {
-			if !app.IsBootstrapped() || app.Settings().Logs.MaxDays == 0 {
-				return nil
+			// don't block and wait for the write transaction to complete
+			// when we can't be sure if the logs write wasn't triggered while
+			// inside another AUX db transaction (ticker or batch threshold reached)
+			// which can block indefinitely and cause deadlock
+			// (https://github.com/pocketbase/pocketbase/issues/7836)
+			shouldBlock, _ := ctx.Value(logger.BlockKey).(bool)
+			if shouldBlock {
+				runLogsWrite(logs)
+			} else {
+				routine.FireAndForget(func() { runLogsWrite(logs) })
 			}
 
-			// write the accumulated logs
-			// (note: based on several local tests there is no significant performance difference between small number of separate write queries vs 1 big INSERT)
-			app.AuxRunInTransaction(func(txApp App) error {
-				model := &Log{}
-				for _, l := range logs {
-					model.MarkAsNew()
-					model.Id = GenerateDefaultRandomId()
-					model.Level = int(l.Level)
-					model.Message = l.Message
-					model.Data = l.Data
-					model.Created, _ = types.ParseDateTime(l.Time)
-
-					if err := txApp.AuxSave(model); err != nil {
-						log.Println("Failed to write log", model, err)
-					}
-				}
-
-				return nil
-			})
-
 			return nil
 		},
 	})
@@ -1535,17 +1586,21 @@ func (app *BaseApp) initLogger() error {
 
 	app.logger = slog.New(handler)
 
-	// write all remaining logs before ticker.Stop to avoid races with ResetBootstrap user calls
-	app.OnTerminate().Bind(&hook.Handler[*TerminateEvent]{
-		Id: "__pbAppLoggerOnTerminate__",
-		Func: func(e *TerminateEvent) error {
-			handler.WriteAll(context.Background())
+	// attempt to write all queued logs before clearing the application bootstrap state
+	app.OnBootstrapClear().Bind(&hook.Handler[*BootstrapEvent]{
+		Id: "__pbAppLoggerFlushBeforeStop__",
+		Func: func(e *BootstrapEvent) error {
+			// extra precaution in case the hook was manually triggered while inside aux db transaction
+			_, isTx := e.App.AuxNonconcurrentDB().(*dbx.Tx)
+			ctx := context.WithValue(context.Background(), logger.BlockKey, !isTx)
+			handler.WriteAll(ctx)
 
+			stopped.Store(true)
 			ticker.Stop()
 
-			// don't block in case OnTerminate is triggered more than once
+			// don't block in case the hook is triggered more than once
 			select {
-			case done <- true:
+			case done <- struct{}{}:
 			default:
 			}