Conversation
The service loaded the DailyRestartNotificationEnabled setting once at startup and never re-read it. When the user unchecked the notification checkbox in the GUI (a separate process), the service still held the old cached value and sent the email anyway. Fix: re-read the notification setting directly from the XML config file (with cache invalidation) right before the notification check in CheckDailyRestart. Also add XMLManager.InvalidateCache() so the service bypasses stale in-memory cache entries written by the GUI process. https://claude.ai/code/session_015AJHHxnPSut7ki1EQEp3C9
The _configCache dictionary only set its expiry when the first item was added, but never cleared old entries on expiry. Each 5-minute cycle added new entries on top of stale ones, causing unbounded growth over the service's uptime. Fix: when the cache has expired, clear all entries before adding the new value and reset the expiry timer. https://claude.ai/code/session_015AJHHxnPSut7ki1EQEp3C9
1. CustomScheduler.InitiateServiceRestart: wrap Process.Start in Using to release the managed handle immediately. The OS process continues running independently — Dispose only frees the .NET wrapper. 2. NotificationManager: change ContinueWith from OnlyOnRanToCompletion to unconditional so _activeBatches entries are always removed, even when the cancellation token fires. Also clear batch.Messages after processing to release EmailMessage references sooner. 3. NotificationManager: remove unused Messages, StartTime, Subject, HighestPriority, BatchId properties and AddMessage/GetCombinedBody methods that duplicated NotificationBatch and were never called — the Messages list in particular grew unbounded. https://claude.ai/code/session_015AJHHxnPSut7ki1EQEp3C9
XMLManager: protect _configCache with SyncLock in InvalidateCache, ReadValue, and WriteValue to prevent races between the scheduler thread and concurrent reads. Remove dead code: - NotificationManager: unused ProcessEmailMessage (superseded by StartEmailProcessor), unused SendEmailAsync (never called), unused disposedValue field (duplicate of _disposed) - CustomScheduler: unused disposedValue, _isProcessing, _processingLock and _schedulerLock (declared/disposed but never acquired) https://claude.ai/code/session_015AJHHxnPSut7ki1EQEp3C9
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR refactors the notification system to use a batch-based approach, improves thread safety in the XML configuration cache, and removes unused code and synchronization primitives.
Key Changes
NotificationManager.vb:
Messages,StartTime,Subject,HighestPriority,BatchId) and related methods (AddMessage,GetCombinedBody,ProcessEmailMessage,SendEmailAsync) in favor of a batch-based architecture usingNotificationBatchclassQueueNotificationto create batches that accumulate messages over 5 seconds before sending_activeBatchesto prevent memory accumulationdisposedValuefield and unnecessaryTaskContinuationOptions.OnlyOnRanToCompletionconstraintXMLManager.vb:
_cacheLocksynchronization object to protect concurrent access to the config cacheInvalidateCache()public method to allow external cache invalidation (useful when GUI updates config)ReadValue()with proper locking around cache checks and updatesWriteValue()with locksCustomScheduler.vb:
disposedValuefield_schedulerLock,_processingLock,_isProcessing)InvalidateCache(), ensuring the service picks up GUI-initiated configuration changesNotable Implementation Details