From eb2cf10a3df3e12ea5702cc62cab978608ef3a42 Mon Sep 17 00:00:00 2001 From: maliming Date: Sat, 21 Mar 2026 15:58:38 +0800 Subject: [PATCH] Enhance dynamic background worker management with improved error handling and logging --- .../DynamicBackgroundJobExecutorJob.cs | 2 +- .../DynamicBackgroundJobHandlerRegistry.cs | 2 +- .../HangfireDynamicBackgroundWorkerManager.cs | 11 ++++++- .../QuartzDynamicBackgroundWorkerAdapter.cs | 9 +++--- .../QuartzDynamicBackgroundWorkerManager.cs | 30 ++++++++++++------- .../IDynamicBackgroundWorkerManager.cs | 7 ++++- 6 files changed, 42 insertions(+), 19 deletions(-) diff --git a/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DynamicBackgroundJobExecutorJob.cs b/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DynamicBackgroundJobExecutorJob.cs index 7462f5b3e5..5818e6de70 100644 --- a/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DynamicBackgroundJobExecutorJob.cs +++ b/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DynamicBackgroundJobExecutorJob.cs @@ -22,7 +22,7 @@ public class DynamicBackgroundJobExecutorJob : AsyncBackgroundJob GetAllNames() { - return Handlers.Keys.ToList(); + return Handlers.Keys.ToList().AsReadOnly(); } public virtual void Clear() diff --git a/framework/src/Volo.Abp.BackgroundWorkers.Hangfire/Volo/Abp/BackgroundWorkers/Hangfire/HangfireDynamicBackgroundWorkerManager.cs b/framework/src/Volo.Abp.BackgroundWorkers.Hangfire/Volo/Abp/BackgroundWorkers/Hangfire/HangfireDynamicBackgroundWorkerManager.cs index 2c297b81a6..6f7e7b325e 100644 --- a/framework/src/Volo.Abp.BackgroundWorkers.Hangfire/Volo/Abp/BackgroundWorkers/Hangfire/HangfireDynamicBackgroundWorkerManager.cs +++ b/framework/src/Volo.Abp.BackgroundWorkers.Hangfire/Volo/Abp/BackgroundWorkers/Hangfire/HangfireDynamicBackgroundWorkerManager.cs @@ -47,8 +47,17 @@ public class HangfireDynamicBackgroundWorkerManager : IDynamicBackgroundWorkerMa cronExpression = GetCron(period); } - ScheduleRecurringJob(workerName, cronExpression, cancellationToken); + // Register the handler first so it is available the moment the recurring job fires. HandlerRegistry.Register(workerName, handler); + try + { + ScheduleRecurringJob(workerName, cronExpression, cancellationToken); + } + catch + { + HandlerRegistry.Unregister(workerName); + throw; + } return Task.CompletedTask; } diff --git a/framework/src/Volo.Abp.BackgroundWorkers.Quartz/Volo/Abp/BackgroundWorkers/Quartz/QuartzDynamicBackgroundWorkerAdapter.cs b/framework/src/Volo.Abp.BackgroundWorkers.Quartz/Volo/Abp/BackgroundWorkers/Quartz/QuartzDynamicBackgroundWorkerAdapter.cs index 6098c461df..d27395bab2 100644 --- a/framework/src/Volo.Abp.BackgroundWorkers.Quartz/Volo/Abp/BackgroundWorkers/Quartz/QuartzDynamicBackgroundWorkerAdapter.cs +++ b/framework/src/Volo.Abp.BackgroundWorkers.Quartz/Volo/Abp/BackgroundWorkers/Quartz/QuartzDynamicBackgroundWorkerAdapter.cs @@ -26,13 +26,14 @@ public class QuartzDynamicBackgroundWorkerAdapter : IJob, ITransientDependency public virtual async Task Execute(IJobExecutionContext context) { - var workerName = context.MergedJobDataMap.GetString(QuartzDynamicBackgroundWorkerManager.DynamicWorkerNameKey); - if (string.IsNullOrWhiteSpace(workerName)) + var rawWorkerName = context.MergedJobDataMap.GetString(QuartzDynamicBackgroundWorkerManager.DynamicWorkerNameKey); + if (string.IsNullOrWhiteSpace(rawWorkerName)) { return; } - var handler = HandlerRegistry.Get(workerName!); + var workerName = rawWorkerName!; + var handler = HandlerRegistry.Get(workerName); if (handler == null) { Logger.LogWarning("No handler registered for dynamic worker: {WorkerName}", workerName); @@ -42,7 +43,7 @@ public class QuartzDynamicBackgroundWorkerAdapter : IJob, ITransientDependency try { await handler( - new DynamicBackgroundWorkerExecutionContext(workerName!, ServiceProvider), + new DynamicBackgroundWorkerExecutionContext(workerName, ServiceProvider), context.CancellationToken); } catch (Exception ex) diff --git a/framework/src/Volo.Abp.BackgroundWorkers.Quartz/Volo/Abp/BackgroundWorkers/Quartz/QuartzDynamicBackgroundWorkerManager.cs b/framework/src/Volo.Abp.BackgroundWorkers.Quartz/Volo/Abp/BackgroundWorkers/Quartz/QuartzDynamicBackgroundWorkerManager.cs index 7065e77f48..f5145de8b6 100644 --- a/framework/src/Volo.Abp.BackgroundWorkers.Quartz/Volo/Abp/BackgroundWorkers/Quartz/QuartzDynamicBackgroundWorkerManager.cs +++ b/framework/src/Volo.Abp.BackgroundWorkers.Quartz/Volo/Abp/BackgroundWorkers/Quartz/QuartzDynamicBackgroundWorkerManager.cs @@ -48,16 +48,24 @@ public class QuartzDynamicBackgroundWorkerManager : IDynamicBackgroundWorkerMana var trigger = BuildTrigger(schedule, jobDetail, triggerKey); - // Use replace=true to avoid TOCTOU race between CheckExists and ScheduleJob. - await Scheduler.ScheduleJobs( - new Dictionary> - { - { jobDetail, new[] { trigger } } - }, - replace: true, - cancellationToken); - + // Register the handler first so it is available the moment the job fires. HandlerRegistry.Register(workerName, handler); + try + { + // Use replace=true to avoid TOCTOU race between CheckExists and ScheduleJob. + await Scheduler.ScheduleJobs( + new Dictionary> + { + { jobDetail, new[] { trigger } } + }, + replace: true, + cancellationToken); + } + catch + { + HandlerRegistry.Unregister(workerName); + throw; + } } public virtual async Task RemoveAsync(string workerName, CancellationToken cancellationToken = default) @@ -68,10 +76,10 @@ public class QuartzDynamicBackgroundWorkerManager : IDynamicBackgroundWorkerMana // This ensures cleanup works correctly after an application restart, when the registry // is empty but the Quartz job may still exist in the scheduler store. var jobKey = new JobKey($"DynamicWorker:{workerName}"); - await Scheduler.DeleteJob(jobKey, cancellationToken); + var deleted = await Scheduler.DeleteJob(jobKey, cancellationToken); var wasRegistered = HandlerRegistry.Unregister(workerName); - return wasRegistered; + return deleted || wasRegistered; } public virtual async Task UpdateScheduleAsync( diff --git a/framework/src/Volo.Abp.BackgroundWorkers/Volo/Abp/BackgroundWorkers/IDynamicBackgroundWorkerManager.cs b/framework/src/Volo.Abp.BackgroundWorkers/Volo/Abp/BackgroundWorkers/IDynamicBackgroundWorkerManager.cs index 34f4d07ccd..7e625e7c9c 100644 --- a/framework/src/Volo.Abp.BackgroundWorkers/Volo/Abp/BackgroundWorkers/IDynamicBackgroundWorkerManager.cs +++ b/framework/src/Volo.Abp.BackgroundWorkers/Volo/Abp/BackgroundWorkers/IDynamicBackgroundWorkerManager.cs @@ -21,7 +21,12 @@ public interface IDynamicBackgroundWorkerManager /// /// Removes a previously added dynamic worker by name. - /// Returns true if the worker was found and removed; false otherwise. + /// Always attempts to remove both the in-memory handler and any persistent scheduling record + /// (Hangfire RecurringJob or Quartz job), regardless of current in-memory state. + /// Returns true if the handler was registered in memory at the time of the call, or if + /// the persistent scheduling record was found and deleted (provider-dependent). + /// May return false after an application restart even if a persistent record was cleaned up, + /// when the provider cannot report whether a persistent record existed (e.g. Hangfire). /// Task RemoveAsync(string workerName, CancellationToken cancellationToken = default);