From 7a1e246ce9c23da1af5c78dc3f33aca05309f92c Mon Sep 17 00:00:00 2001 From: maliming Date: Sat, 21 Mar 2026 15:49:03 +0800 Subject: [PATCH] Enhance dynamic background job handling with improved argument management and error logging --- .../DynamicBackgroundJobArgs.cs | 11 +++++-- .../DynamicBackgroundJobHandlerRegistry.cs | 12 +++++++ .../IDynamicBackgroundJobHandlerRegistry.cs | 6 ++++ .../HangfireBackgroundWorkerManager.cs | 12 ++++--- .../HangfireDynamicBackgroundWorkerAdapter.cs | 5 ++- .../HangfireDynamicBackgroundWorkerManager.cs | 24 +++++++------- .../QuartzDynamicBackgroundWorkerAdapter.cs | 5 ++- .../QuartzDynamicBackgroundWorkerManager.cs | 31 +++++++++---------- .../DefaultDynamicBackgroundWorkerManager.cs | 10 ++++++ 9 files changed, 80 insertions(+), 36 deletions(-) diff --git a/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DynamicBackgroundJobArgs.cs b/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DynamicBackgroundJobArgs.cs index 9eaf2b00b2..45066b7586 100644 --- a/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DynamicBackgroundJobArgs.cs +++ b/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DynamicBackgroundJobArgs.cs @@ -5,9 +5,16 @@ public class DynamicBackgroundJobArgs { public const string JobNameConstant = "Abp.DynamicJob"; - public string JobName { get; } + public string JobName { get; private set; } - public string JsonData { get; } + public string JsonData { get; private set; } + + // For serializers that require a parameterless constructor (e.g. System.Text.Json) + private DynamicBackgroundJobArgs() + { + JobName = string.Empty; + JsonData = string.Empty; + } public DynamicBackgroundJobArgs(string jobName, string jsonData) { diff --git a/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DynamicBackgroundJobHandlerRegistry.cs b/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DynamicBackgroundJobHandlerRegistry.cs index ce5710dd14..1c12300ffd 100644 --- a/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DynamicBackgroundJobHandlerRegistry.cs +++ b/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DynamicBackgroundJobHandlerRegistry.cs @@ -1,4 +1,6 @@ using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; using Volo.Abp.DependencyInjection; namespace Volo.Abp.BackgroundJobs; @@ -37,4 +39,14 @@ public class DynamicBackgroundJobHandlerRegistry : IDynamicBackgroundJobHandlerR Check.NotNullOrWhiteSpace(jobName, nameof(jobName)); return Handlers.TryGetValue(jobName, out var handler) ? handler : null; } + + public virtual IReadOnlyCollection GetAllNames() + { + return Handlers.Keys.ToList(); + } + + public virtual void Clear() + { + Handlers.Clear(); + } } diff --git a/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/IDynamicBackgroundJobHandlerRegistry.cs b/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/IDynamicBackgroundJobHandlerRegistry.cs index dff212d65b..57e75d1607 100644 --- a/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/IDynamicBackgroundJobHandlerRegistry.cs +++ b/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/IDynamicBackgroundJobHandlerRegistry.cs @@ -1,3 +1,5 @@ +using System.Collections.Generic; + namespace Volo.Abp.BackgroundJobs; public interface IDynamicBackgroundJobHandlerRegistry @@ -9,4 +11,8 @@ public interface IDynamicBackgroundJobHandlerRegistry bool IsRegistered(string jobName); DynamicBackgroundJobHandler? Get(string jobName); + + IReadOnlyCollection GetAllNames(); + + void Clear(); } diff --git a/framework/src/Volo.Abp.BackgroundWorkers.Hangfire/Volo/Abp/BackgroundWorkers/Hangfire/HangfireBackgroundWorkerManager.cs b/framework/src/Volo.Abp.BackgroundWorkers.Hangfire/Volo/Abp/BackgroundWorkers/Hangfire/HangfireBackgroundWorkerManager.cs index 64a4a1be64..e23737d2a1 100644 --- a/framework/src/Volo.Abp.BackgroundWorkers.Hangfire/Volo/Abp/BackgroundWorkers/Hangfire/HangfireBackgroundWorkerManager.cs +++ b/framework/src/Volo.Abp.BackgroundWorkers.Hangfire/Volo/Abp/BackgroundWorkers/Hangfire/HangfireBackgroundWorkerManager.cs @@ -162,19 +162,23 @@ public class HangfireBackgroundWorkerManager : BackgroundWorkerManager, ISinglet if (time.TotalSeconds <= 59) { - cron = $"*/{time.TotalSeconds} * * * * *"; + var seconds = Math.Max(1, (int)Math.Round(time.TotalSeconds)); + cron = $"*/{seconds} * * * * *"; } else if (time.TotalMinutes <= 59) { - cron = $"*/{time.TotalMinutes} * * * *"; + var minutes = Math.Max(1, (int)Math.Round(time.TotalMinutes)); + cron = $"*/{minutes} * * * *"; } else if (time.TotalHours <= 23) { - cron = $"0 */{time.TotalHours} * * *"; + var hours = Math.Max(1, (int)Math.Round(time.TotalHours)); + cron = $"0 */{hours} * * *"; } else if(time.TotalDays <= 31) { - cron = $"0 0 0 1/{time.TotalDays} * *"; + var days = Math.Max(1, (int)Math.Round(time.TotalDays)); + cron = $"0 0 0 1/{days} * *"; } else { diff --git a/framework/src/Volo.Abp.BackgroundWorkers.Hangfire/Volo/Abp/BackgroundWorkers/Hangfire/HangfireDynamicBackgroundWorkerAdapter.cs b/framework/src/Volo.Abp.BackgroundWorkers.Hangfire/Volo/Abp/BackgroundWorkers/Hangfire/HangfireDynamicBackgroundWorkerAdapter.cs index 9e6d5f2f0c..56a5d7a5c8 100644 --- a/framework/src/Volo.Abp.BackgroundWorkers.Hangfire/Volo/Abp/BackgroundWorkers/Hangfire/HangfireDynamicBackgroundWorkerAdapter.cs +++ b/framework/src/Volo.Abp.BackgroundWorkers.Hangfire/Volo/Abp/BackgroundWorkers/Hangfire/HangfireDynamicBackgroundWorkerAdapter.cs @@ -39,10 +39,13 @@ public class HangfireDynamicBackgroundWorkerAdapter : ITransientDependency } catch (Exception ex) { + // Swallow the exception to match the behavior of AsyncPeriodicBackgroundWorkerBase, + // which catches, notifies and logs without rethrowing. This prevents Hangfire from + // treating a single failed execution as a job failure and triggering retries. await ServiceProvider.GetRequiredService() .NotifyAsync(new ExceptionNotificationContext(ex)); - throw; + Logger.LogException(ex); } } } 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 f34237fcc0..2c297b81a6 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 @@ -57,16 +57,14 @@ public class HangfireDynamicBackgroundWorkerManager : IDynamicBackgroundWorkerMa { Check.NotNullOrWhiteSpace(workerName, nameof(workerName)); - if (!HandlerRegistry.IsRegistered(workerName)) - { - return Task.FromResult(false); - } - + // Always remove the persistent recurring job regardless of in-memory registry state. + // This ensures cleanup works correctly after an application restart, when the registry + // is empty but the Hangfire recurring job may still exist in the database. var recurringJobId = $"DynamicWorker:{workerName}"; RecurringJob.RemoveIfExists(recurringJobId); - HandlerRegistry.Unregister(workerName); + var wasRegistered = HandlerRegistry.Unregister(workerName); - return Task.FromResult(true); + return Task.FromResult(wasRegistered); } public virtual Task UpdateScheduleAsync( @@ -150,19 +148,23 @@ public class HangfireDynamicBackgroundWorkerManager : IDynamicBackgroundWorkerMa if (time.TotalSeconds <= 59) { - cron = $"*/{time.TotalSeconds} * * * * *"; + var seconds = Math.Max(1, (int)Math.Round(time.TotalSeconds)); + cron = $"*/{seconds} * * * * *"; } else if (time.TotalMinutes <= 59) { - cron = $"*/{time.TotalMinutes} * * * *"; + var minutes = Math.Max(1, (int)Math.Round(time.TotalMinutes)); + cron = $"*/{minutes} * * * *"; } else if (time.TotalHours <= 23) { - cron = $"0 */{time.TotalHours} * * *"; + var hours = Math.Max(1, (int)Math.Round(time.TotalHours)); + cron = $"0 */{hours} * * *"; } else if (time.TotalDays <= 31) { - cron = $"0 0 0 1/{time.TotalDays} * *"; + var days = Math.Max(1, (int)Math.Round(time.TotalDays)); + cron = $"0 0 0 1/{days} * *"; } else { 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 ba8d98091f..6098c461df 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 @@ -47,10 +47,13 @@ public class QuartzDynamicBackgroundWorkerAdapter : IJob, ITransientDependency } catch (Exception ex) { + // Swallow the exception to match the behavior of AsyncPeriodicBackgroundWorkerBase, + // which catches, notifies and logs without rethrowing. This prevents Quartz from + // treating a single failed execution as a job failure and triggering retries. await ServiceProvider.GetRequiredService() .NotifyAsync(new ExceptionNotificationContext(ex)); - throw; + Logger.LogException(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 4c8b643782..598c506976 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 @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging; @@ -47,16 +48,14 @@ public class QuartzDynamicBackgroundWorkerManager : IDynamicBackgroundWorkerMana var trigger = BuildTrigger(schedule, jobDetail, triggerKey); - if (await Scheduler.CheckExists(jobDetail.Key, cancellationToken)) - { - await Scheduler.AddJob(jobDetail, true, true, cancellationToken); - await Scheduler.ResumeJob(jobDetail.Key, cancellationToken); - await Scheduler.RescheduleJob(trigger.Key, trigger, cancellationToken); - } - else - { - await Scheduler.ScheduleJob(jobDetail, trigger, cancellationToken); - } + // Use replace=true to avoid TOCTOU race between CheckExists and ScheduleJob. + await Scheduler.ScheduleJobs( + new Dictionary> + { + { jobDetail, new[] { trigger } } + }, + replace: true, + cancellationToken); HandlerRegistry.Register(workerName, handler); } @@ -65,16 +64,14 @@ public class QuartzDynamicBackgroundWorkerManager : IDynamicBackgroundWorkerMana { Check.NotNullOrWhiteSpace(workerName, nameof(workerName)); - if (!HandlerRegistry.IsRegistered(workerName)) - { - return false; - } - + // Always delete the persistent Quartz job regardless of in-memory registry state. + // 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); HandlerRegistry.Unregister(workerName); - return true; + return deleted; } public virtual async Task UpdateScheduleAsync( diff --git a/framework/src/Volo.Abp.BackgroundWorkers/Volo/Abp/BackgroundWorkers/DefaultDynamicBackgroundWorkerManager.cs b/framework/src/Volo.Abp.BackgroundWorkers/Volo/Abp/BackgroundWorkers/DefaultDynamicBackgroundWorkerManager.cs index 4116baa6c2..683d42687f 100644 --- a/framework/src/Volo.Abp.BackgroundWorkers/Volo/Abp/BackgroundWorkers/DefaultDynamicBackgroundWorkerManager.cs +++ b/framework/src/Volo.Abp.BackgroundWorkers/Volo/Abp/BackgroundWorkers/DefaultDynamicBackgroundWorkerManager.cs @@ -49,6 +49,11 @@ public class DefaultDynamicBackgroundWorkerManager : IDynamicBackgroundWorkerMan await _semaphore.WaitAsync(cancellationToken); try { + if (_isDisposed) + { + throw new ObjectDisposedException(nameof(DefaultDynamicBackgroundWorkerManager)); + } + if (_dynamicWorkers.TryRemove(workerName, out var existingWorker)) { await existingWorker.StopAsync(cancellationToken); @@ -107,6 +112,11 @@ public class DefaultDynamicBackgroundWorkerManager : IDynamicBackgroundWorkerMan await _semaphore.WaitAsync(cancellationToken); try { + if (_isDisposed) + { + throw new ObjectDisposedException(nameof(DefaultDynamicBackgroundWorkerManager)); + } + if (!_dynamicWorkers.TryGetValue(workerName, out var worker)) { return false;