fix: memory leak for cronjobs #356

backend/cronjobs/cronjobs.go

@@ -8,21 +8,24 @@ import (
 	"github.com/seriousm4x/upsnap/networking"
 )
 
-var PingRunning bool = false
-var WakeShutdownRunning bool = false
-var CronPing *cron.Cron
-var CronWakeShutdown *cron.Cron
+var (
+	PingRunning         = false
+	WakeShutdownRunning = false
+	CronPing            = cron.New()
+	CronWakeShutdown    = cron.New()
+)
 
-func RunPing(app *pocketbase.PocketBase) {
-	PingRunning = true
+func SetPingJobs(app *pocketbase.PocketBase) {
+	// remove existing jobs
+	for _, job := range CronPing.Entries() {
+		CronPing.Remove(job.ID)
+	}
 
 	settingsPrivateRecords, err := app.Dao().FindRecordsByExpr("settings_private")
 	if err != nil {
 		logger.Error.Println(err)
 	}
 
-	// init cronjob
-	CronPing = cron.New()
 	CronPing.AddFunc(settingsPrivateRecords[0].GetString("interval"), func() {
 		// skip cron if no realtime clients connected and lazy_ping is turned on
 		realtimeClients := len(app.SubscriptionsBroker().Clients())
@@ -89,13 +92,14 @@ func RunPing(app *pocketbase.PocketBase) {
 			}(device)
 		}
 	})
-	CronPing.Run()
 }
 
-func RunWakeShutdown(app *pocketbase.PocketBase) {
-	WakeShutdownRunning = true
+func SetWakeShutdownJobs(app *pocketbase.PocketBase) {
+	// remove existing jobs
+	for _, job := range CronWakeShutdown.Entries() {
+		CronWakeShutdown.Remove(job.ID)
+	}
 
-	CronWakeShutdown = cron.New()
 	devices, err := app.Dao().FindRecordsByExpr("devices")
 	if err != nil {
 		logger.Error.Println(err)
@@ -109,70 +113,93 @@ func RunWakeShutdown(app *pocketbase.PocketBase) {
 		shutdown_cron_enabled := dev.GetBool("shutdown_cron_enabled")
 
 		if wake_cron_enabled && wake_cron != "" {
-			go func(d *models.Record) {
-				_, err := CronWakeShutdown.AddFunc(wake_cron, func() {
-					d.Set("status", "pending")
-					if err := app.Dao().SaveRecord(d); err != nil {
+			_, err := CronWakeShutdown.AddFunc(wake_cron, func() {
+				status := dev.GetString("status")
+				if status == "pending" || status == "online" {
+					return
+				}
+				dev.Set("status", "pending")
+				if err := app.Dao().SaveRecord(dev); err != nil {
+					logger.Error.Println("Failed to save record:", err)
+				}
+				if err := networking.WakeDevice(dev); err != nil {
+					logger.Error.Println(err)
+					dev.Set("status", "offline")
+					if err := app.Dao().SaveRecord(dev); err != nil {
 						logger.Error.Println("Failed to save record:", err)
 					}
-					if err := networking.WakeDevice(d); err != nil {
-						logger.Error.Println(err)
-						d.Set("status", "offline")
-						if err := app.Dao().SaveRecord(d); err != nil {
-							logger.Error.Println("Failed to save record:", err)
-						}
-					} else {
-						d.Set("status", "online")
-						if err := app.Dao().SaveRecord(d); err != nil {
-							logger.Error.Println("Failed to save record:", err)
-						}
+				} else {
+					dev.Set("status", "online")
+					if err := app.Dao().SaveRecord(dev); err != nil {
+						logger.Error.Println("Failed to save record:", err)
 					}
-				})
-				if err != nil {
-					logger.Error.Println(err)
 				}
-			}(dev)
+			})
+			if err != nil {
+				logger.Error.Println(err)
+			}
 		}
 
 		if shutdown_cron_enabled && shutdown_cron != "" {
-			go func(d *models.Record) {
-				_, err := CronWakeShutdown.AddFunc(shutdown_cron, func() {
-					d.Set("status", "pending")
-					if err := app.Dao().SaveRecord(d); err != nil {
+			_, err := CronWakeShutdown.AddFunc(shutdown_cron, func() {
+				status := dev.GetString("status")
+				if status == "pending" || status == "offline" {
+					return
+				}
+				dev.Set("status", "pending")
+				if err := app.Dao().SaveRecord(dev); err != nil {
+					logger.Error.Println("Failed to save record:", err)
+				}
+				if err := networking.ShutdownDevice(dev); err != nil {
+					logger.Error.Println(err)
+					dev.Set("status", "online")
+					if err := app.Dao().SaveRecord(dev); err != nil {
 						logger.Error.Println("Failed to save record:", err)
 					}
-					if err := networking.ShutdownDevice(d); err != nil {
-						logger.Error.Println(err)
-						d.Set("status", "online")
-						if err := app.Dao().SaveRecord(d); err != nil {
-							logger.Error.Println("Failed to save record:", err)
-						}
-					} else {
-						d.Set("status", "offline")
-						if err := app.Dao().SaveRecord(d); err != nil {
-							logger.Error.Println("Failed to save record:", err)
-						}
+				} else {
+					dev.Set("status", "offline")
+					if err := app.Dao().SaveRecord(dev); err != nil {
+						logger.Error.Println("Failed to save record:", err)
 					}
-				})
-				if err != nil {
-					logger.Error.Println(err)
 				}
-			}(dev)
+			})
+			if err != nil {
+				logger.Error.Println(err)
+			}
 		}
 	}
-	CronWakeShutdown.Run()
 }
 
-func StopAll() {
-	if PingRunning {
-		logger.Info.Println("Stopping ping cronjob")
-		ctx := CronPing.Stop()
-		<-ctx.Done()
+func StartWakeShutdown() {
+	WakeShutdownRunning = true
+	go CronWakeShutdown.Run()
 
-	}
+}
+
+func StopWakeShutdown() {
 	if WakeShutdownRunning {
 		logger.Info.Println("Stopping wake/shutdown cronjob")
 		ctx := CronWakeShutdown.Stop()
 		<-ctx.Done()
 	}
+	WakeShutdownRunning = false
+}
+
+func StartPing() {
+	PingRunning = true
+	go CronPing.Run()
+}
+
+func StopPing() {
+	if PingRunning {
+		logger.Info.Println("Stopping wake/shutdown cronjob")
+		ctx := CronPing.Stop()
+		<-ctx.Done()
+	}
+	PingRunning = false
+}
+
+func StopAll() {
+	StopPing()
+	StopWakeShutdown()
 }

backend/pb/pb.go

@@ -122,23 +122,37 @@ func StartPocketBase(distDirFS fs.FS) {
 			return err
 		}
 
-		go cronjobs.RunPing(App)
-		go cronjobs.RunWakeShutdown(App)
+		cronjobs.SetPingJobs(App)
+		cronjobs.StartPing()
+		cronjobs.SetWakeShutdownJobs(App)
+		cronjobs.StartWakeShutdown()
 
 		// restart ping cronjobs or wake/shutdown cronjobs on model update
 		// add event hook before starting server.
 		// using this outside App.OnBeforeServe() would not work
 		App.OnModelAfterUpdate("settings_private", "devices").Add(func(e *core.ModelEvent) error {
 			if e.Model.TableName() == "settings_private" {
-				for _, job := range cronjobs.CronPing.Entries() {
-					cronjobs.CronPing.Remove(job.ID)
-				}
-				go cronjobs.RunPing(App)
+				cronjobs.SetPingJobs(App)
 			} else if e.Model.TableName() == "devices" {
-				for _, job := range cronjobs.CronWakeShutdown.Entries() {
-					cronjobs.CronWakeShutdown.Remove(job.ID)
+				// only restart wake/shutdown cronjobs if new model's cron changed
+				newRecord := e.Model.(*models.Record)
+				newWakeCron := newRecord.GetString("wake_cron")
+				newWakeCronEnabled := newRecord.GetBool("wake_cron_enabled")
+				newShutdownCron := newRecord.GetString("shutdown_cron")
+				newShutdownCronEnabled := newRecord.GetBool("shutdown_cron_enabled")
+
+				oldRecord := newRecord.OriginalCopy()
+				oldWakeCron := oldRecord.GetString("wake_cron")
+				oldWakeCronEnabled := oldRecord.GetBool("wake_cron_enabled")
+				oldShutdownCron := oldRecord.GetString("shutdown_cron")
+				oldShutdownCronEnabled := oldRecord.GetBool("shutdown_cron_enabled")
+
+				if newWakeCron != oldWakeCron ||
+					newWakeCronEnabled != oldWakeCronEnabled ||
+					newShutdownCron != oldShutdownCron ||
+					newShutdownCronEnabled != oldShutdownCronEnabled {
+					cronjobs.SetWakeShutdownJobs(App)
 				}
-				go cronjobs.RunWakeShutdown(App)
 			}
 			return nil
 		})