From e2fab5695bedaf0aaff280251225921e817b18be Mon Sep 17 00:00:00 2001 From: Sami Alzein Date: Wed, 6 Aug 2025 21:36:41 +0200 Subject: [PATCH] Add detailed threading and concurrency specification documentation; outline architecture, thread hierarchy, specifications, and identified concurrency issues --- docs/06-threading-concurrency.md | 541 +++++++++++++++++++++++++++++++ 1 file changed, 541 insertions(+) create mode 100644 docs/06-threading-concurrency.md diff --git a/docs/06-threading-concurrency.md b/docs/06-threading-concurrency.md new file mode 100644 index 0000000..727b75e --- /dev/null +++ b/docs/06-threading-concurrency.md @@ -0,0 +1,541 @@ +# Threading and Concurrency Specification + +## Overview + +The tempering machine control system employs a complex multi-threaded architecture with 6 concurrent background threads handling different aspects of system operation. This document details the threading model, synchronization mechanisms, and concurrency issues. + +## Thread Architecture + +### Thread Hierarchy + +``` +Application Process (MainWindow) +├── UI Thread (Primary) +│ ├── Avalonia UI Updates +│ ├── User Input Handling +│ └── Event Dispatching +│ +├── MonitorPortsLoop Thread ⭐ (Highest Priority) +│ ├── Hardware Communication +│ ├── Recipe Phase Management +│ ├── Temperature Monitoring +│ ├── Motor Control Logic +│ └── Safety System Monitoring +│ +├── SerialThreadLoop Thread +│ ├── Modbus Communication Queue +│ ├── Message Retry Logic +│ ├── CRC Validation +│ └── Timeout Management +│ +├── ScreenLoop Thread +│ ├── Error State Management +│ ├── UI Element Enable/Disable +│ ├── Error Display Logic +│ └── Button State Control +│ +├── InteractiveUILoop Thread +│ ├── Visual Feedback (Flashing) +│ ├── Status Indicators +│ └── Animation Updates +│ +├── TouchLoop Thread +│ ├── Activity Tracking +│ ├── Screen Dimming Control +│ └── Power Management +│ +└── CheckInternetLoop Thread + ├── Network Connectivity + ├── Internet Access Validation + └── Connection Status Updates +``` + +## Thread Specifications + +### 1. MonitorPortsLoop Thread (Critical) + +``` +Thread Configuration: +┌─────────────────────────────────────────────────────────────┐ +│ Name: MonitorPortsLoop │ +│ Priority: ThreadPriority.Highest │ +│ Type: Background Thread │ +│ Frequency: Continuous loop with 100ms intervals │ +│ │ +│ Primary Responsibilities: │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ • Hardware State Monitoring │ │ +│ │ • Recipe Phase Transitions │ │ +│ │ • Temperature Control Logic │ │ +│ │ • Motor Coordination │ │ +│ │ • Safety System Enforcement │ │ +│ │ • Error Condition Detection │ │ +│ │ • Timer Management (Heating/Cooling/Pouring) │ │ +│ │ • Pedal Control (Manual/Auto modes) │ │ +│ └─────────────────────────────────────────────────────────┘ │ +│ │ +│ Shared State Access: │ +│ • holdingRegister (Read/Write) │ +│ • Temperature variables (comTankTemp, comFountainTemp) │ +│ • Recipe state variables (Heating, cooling, pouring) │ +│ • Motor state flags (isMixerMotorOn, isFountainMotorOn) │ +│ • Timer objects (heatingTimer, coolingTimer, etc.) │ +└─────────────────────────────────────────────────────────────┘ +``` + +### 2. SerialThreadLoop Thread (Communication) + +``` +Thread Configuration: +┌─────────────────────────────────────────────────────────────┐ +│ Name: SerialThreadLoop │ +│ Priority: Normal │ +│ Type: Background Thread │ +│ Frequency: Event-driven with timing controls │ +│ │ +│ Communication Pattern: │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ Queue Management: │ │ +│ │ ┌─────────────┐ ┌─────────────┐ │ │ +│ │ │ Write Queue │ │ Read Queue │ │ │ +│ │ │ (Priority) │ │ (Periodic) │ │ │ +│ │ │ │ │ │ │ │ +│ │ │ • Motor │ │ • Temp │ │ │ +│ │ │ commands │ │ readings │ │ │ +│ │ │ • Temp │ │ • Status │ │ │ +│ │ │ setpoints │ │ updates │ │ │ +│ │ │ • Config │ │ • Input │ │ │ +│ │ │ updates │ │ registers │ │ │ +│ │ └─────────────┘ └─────────────┘ │ │ +│ └─────────────────────────────────────────────────────────┘ │ +│ │ +│ Timing Control: │ +│ • Minimum interval: screenData.sendingTime (100-500ms) │ +│ • Retry attempts: 3 with exponential backoff │ +│ • CRC validation on all messages │ +│ • Timeout handling: 3 seconds per operation │ +└─────────────────────────────────────────────────────────────┘ +``` + +### 3. ScreenLoop Thread (UI Management) + +``` +Thread Configuration: +┌─────────────────────────────────────────────────────────────┐ +│ Name: ScreenLoop │ +│ Priority: Normal │ +│ Type: Background Thread │ +│ Frequency: 10ms intervals (high frequency monitoring) │ +│ │ +│ Error Management Logic: │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ Error Detection (2.5s delay): │ │ +│ │ ┌─────────────┐ ┌─────────────┐ ┌─────────────┐ │ │ +│ │ │ Error │───▶│ Display │───▶│ Action │ │ │ +│ │ │ Triggered │ │ Message │ │ Disable │ │ │ +│ │ │ │ │ │ │ Controls │ │ │ +│ │ └─────────────┘ └─────────────┘ └─────────────┘ │ │ +│ │ │ │ +│ │ Error Recovery (automatic): │ │ +│ │ ┌─────────────┐ ┌─────────────┐ ┌─────────────┐ │ │ +│ │ │ Condition │───▶│ Clear │───▶│ Re-enable │ │ │ +│ │ │ Cleared │ │ Message │ │ Controls │ │ │ +│ │ │ │ │ │ │ │ │ │ +│ │ └─────────────┘ └─────────────┘ └─────────────┘ │ │ +│ └─────────────────────────────────────────────────────────┘ │ +│ │ +│ UI State Synchronization: │ +│ • Uses Dispatcher.UIThread.Post() for thread safety │ +│ • Manages button enable/disable states │ +│ • Controls error popup visibility │ +│ • Handles recipe pause/resume logic │ +└─────────────────────────────────────────────────────────────┘ +``` + +## Concurrency Issues and Problems + +### Critical Race Conditions + +``` +Race Condition Analysis: +┌─────────────────────────────────────────────────────────────┐ +│ IDENTIFIED ISSUES │ +│ │ +│ 1. Shared Variable Access (HIGH RISK): │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ Variables accessed by multiple threads: │ │ +│ │ • holdingRegister.* (MonitorPortsLoop + SerialThread) │ │ +│ │ • Temperature variables (MonitorPortsLoop + UI) │ │ +│ │ • Motor state flags (MonitorPortsLoop + ScreenLoop) │ │ +│ │ • Recipe state variables (MonitorPortsLoop + UI) │ │ +│ │ • Timer objects (MonitorPortsLoop + UI Events) │ │ +│ │ │ │ +│ │ Risk: Data corruption, inconsistent states │ │ +│ │ Current Protection: NONE (No locks or synchronization) │ │ +│ └─────────────────────────────────────────────────────────┘ │ +│ │ +│ 2. Timer Object Management (MEDIUM RISK): │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ Multiple threads can: │ │ +│ │ • Create new timers │ │ +│ │ • Dispose existing timers │ │ +│ │ • Change timer intervals │ │ +│ │ • Reset timer states │ │ +│ │ │ │ +│ │ Risk: Memory leaks, timer conflicts, invalid states │ │ +│ │ Current Protection: Partial (null checks only) │ │ +│ └─────────────────────────────────────────────────────────┘ │ +│ │ +│ 3. Error Collection Access (MEDIUM RISK): │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ static List errors accessed by: │ │ +│ │ • MonitorPortsLoop (Add/Remove errors) │ │ +│ │ • ScreenLoop (Display/Clear errors) │ │ +│ │ • Multiple UI threads (Error status checks) │ │ +│ │ │ │ +│ │ Risk: Collection modification exceptions, data loss │ │ +│ │ Current Protection: ToList() cloning in some places │ │ +│ └─────────────────────────────────────────────────────────┘ │ +└─────────────────────────────────────────────────────────────┘ +``` + +### Memory Management Issues + +``` +Resource Management Problems: +┌─────────────────────────────────────────────────────────────┐ +│ MEMORY LEAK SOURCES │ +│ │ +│ 1. Timer Objects: │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ Created: heatingTimer, coolingTimer, pouringTimer, │ │ +│ │ mixerTimer, fountainTimer, pedalTimers │ │ +│ │ │ │ +│ │ Issues: │ │ +│ │ • Not always properly disposed │ │ +│ │ • Can be recreated without disposing previous │ │ +│ │ • Exception handling may skip disposal │ │ +│ │ • Background threads may create orphaned timers │ │ +│ └─────────────────────────────────────────────────────────┘ │ +│ │ +│ 2. Event Handlers: │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ UI events registered but not unregistered: │ │ +│ │ • Button click handlers │ │ +│ │ • Timer elapsed events │ │ +│ │ • Dispatcher callbacks │ │ +│ │ │ │ +│ │ Risk: Event handler accumulation, memory growth │ │ +│ └─────────────────────────────────────────────────────────┘ │ +│ │ +│ 3. Serial Port Resources: │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ SerialPort objects may not be properly closed: │ │ +│ │ • Exception during port operations │ │ +│ │ • Thread termination without cleanup │ │ +│ │ • Port reset without proper disposal │ │ +│ └─────────────────────────────────────────────────────────┘ │ +└─────────────────────────────────────────────────────────────┘ +``` + +## Synchronization Mechanisms + +### Current Approach (Inadequate) + +``` +Current Synchronization: +┌─────────────────────────────────────────────────────────────┐ +│ EXISTING PATTERNS │ +│ │ +│ 1. UI Thread Dispatching: │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ Dispatcher.UIThread.Post(() => { │ │ +│ │ // UI updates from background threads │ │ +│ │ footerMsg.Text = "Status message"; │ │ +│ │ }); │ │ +│ │ │ │ +│ │ ✓ Prevents cross-thread UI access violations │ │ +│ │ ✗ No protection for shared business logic state │ │ +│ └─────────────────────────────────────────────────────────┘ │ +│ │ +│ 2. Serial Communication Queue: │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ ConcurrentQueue for message queuing │ │ +│ │ │ │ +│ │ ✓ Thread-safe message queuing │ │ +│ │ ✗ No protection for shared state accessed by handlers │ │ +│ └─────────────────────────────────────────────────────────┘ │ +│ │ +│ 3. Semaphore Usage (Limited): │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ SemaphoreSlim _keepSendingLock = new(1, 1); │ │ +│ │ │ │ +│ │ ✓ Protects some serial communication operations │ │ +│ │ ✗ Not used for other shared state protection │ │ +│ └─────────────────────────────────────────────────────────┘ │ +└─────────────────────────────────────────────────────────────┘ +``` + +### Recommended Synchronization Strategy + +``` +Improved Synchronization Design: +┌─────────────────────────────────────────────────────────────┐ +│ PROTECTION STRATEGIES │ +│ │ +│ 1. State Object Locking: │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ class ThreadSafeHoldingRegister { │ │ +│ │ private readonly object _lock = new object(); │ │ +│ │ private HoldingRegister _register; │ │ +│ │ │ │ +│ │ public void UpdateMotor(ushort value) { │ │ +│ │ lock (_lock) { │ │ +│ │ _register.motor = value; │ │ +│ │ } │ │ +│ │ } │ │ +│ │ } │ │ +│ └─────────────────────────────────────────────────────────┘ │ +│ │ +│ 2. Reader-Writer Locks for Temperature Data: │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ ReaderWriterLockSlim _tempLock = new(); │ │ +│ │ │ │ +│ │ // Read operations (frequent) │ │ +│ │ _tempLock.EnterReadLock(); │ │ +│ │ var temp = comTankTemp; │ │ +│ │ _tempLock.ExitReadLock(); │ │ +│ │ │ │ +│ │ // Write operations (less frequent) │ │ +│ │ _tempLock.EnterWriteLock(); │ │ +│ │ comTankTemp = newValue; │ │ +│ │ _tempLock.ExitWriteLock(); │ │ +│ └─────────────────────────────────────────────────────────┘ │ +│ │ +│ 3. Concurrent Collections for Errors: │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ ConcurrentBag errors = new(); │ │ +│ │ │ │ +│ │ // Thread-safe add/remove operations │ │ +│ │ errors.Add(newError); │ │ +│ │ │ │ +│ │ // Safe enumeration with snapshot │ │ +│ │ var currentErrors = errors.ToArray(); │ │ +│ └─────────────────────────────────────────────────────────┘ │ +└─────────────────────────────────────────────────────────────┘ +``` + +## Thread Communication Patterns + +### Message Passing Architecture + +``` +Inter-Thread Communication: +┌─────────────────────────────────────────────────────────────┐ +│ MESSAGE FLOW │ +│ │ +│ ┌─────────────────┐ ┌─────────────────┐ │ +│ │ MonitorPortsLoop│───▶│ SerialThreadLoop│ │ +│ │ │ │ │ │ +│ │ • Motor commands│ │ • Queue mgmt │ │ +│ │ • Temp setpoints│ │ • Retry logic │ │ +│ │ • Config updates│ │ • CRC validation│ │ +│ └─────────────────┘ └─────────────────┘ │ +│ │ │ │ +│ ▼ ▼ │ +│ ┌─────────────────┐ ┌─────────────────┐ │ +│ │ ScreenLoop │───▶│ UI Thread │ │ +│ │ │ │ │ │ +│ │ • Error states │ │ • Visual updates│ │ +│ │ • Button control│ │ • User feedback │ │ +│ │ • Status updates│ │ • Event handling│ │ +│ └─────────────────┘ └─────────────────┘ │ +│ │ +│ Communication Mechanisms: │ +│ • Shared memory (current - problematic) │ +│ • Event-driven callbacks │ +│ • Queue-based messaging (serial only) │ +│ • Direct method calls (thread-unsafe) │ +└─────────────────────────────────────────────────────────────┘ +``` + +### Recommended Event-Driven Pattern + +``` +Event-Based Communication: +┌─────────────────────────────────────────────────────────────┐ +│ EVENT SYSTEM │ +│ │ +│ ┌─────────────────┐ Events ┌─────────────────────────┐ │ +│ │ Publishers │◄──────────── ▶│ Event Bus │ │ +│ │ │ │ │ │ +│ │ • Temperature │ │ • Thread-safe routing │ │ +│ │ Controller │ │ • Event queuing │ │ +│ │ • Motor │ │ • Priority handling │ │ +│ │ Controller │ │ • Error propagation │ │ +│ │ • Recipe │ └─────────────────────────┘ │ +│ │ Manager │ │ │ +│ └─────────────────┘ ▼ │ +│ ┌─────────────────────────┐ │ +│ │ Subscribers │ │ +│ │ │ │ +│ │ • UI Controllers │ │ +│ │ • Error Handlers │ │ +│ │ • Data Loggers │ │ +│ │ • Safety Systems │ │ +│ └─────────────────────────┘ │ +│ │ +│ Event Types: │ +│ • TemperatureChanged(zone, value) │ +│ • MotorStateChanged(motor, state) │ +│ • RecipePhaseChanged(phase, progress) │ +│ • ErrorOccurred(error, severity) │ +│ • ErrorCleared(error) │ +│ • CommunicationStatus(connected, quality) │ +└─────────────────────────────────────────────────────────────┘ +``` + +## Thread Lifecycle Management + +### Startup Sequence + +``` +Application Startup: +┌─────────────────────────────────────────────────────────────┐ +│ MainWindow Constructor │ +│ │ │ +│ ▼ │ +│ ┌─────────────────┐ │ +│ │ Initialize UI │ │ +│ │ Components │ │ +│ └─────────────────┘ │ +│ │ │ +│ ▼ │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ Start Background Threads: │ │ +│ │ │ │ +│ │ 1. serialThread = new Thread(serialThreadLoop.Send) │ │ +│ │ serialThread.IsBackground = true │ │ +│ │ serialThread.Start() │ │ +│ │ │ │ +│ │ 2. internetThread = new Thread(CheckInterNet) │ │ +│ │ internetThread.IsBackground = true │ │ +│ │ internetThread.Start() │ │ +│ │ │ │ +│ │ 3. screenThread = new Thread(ScreenLoop.Screen) │ │ +│ │ screenThread.IsBackground = true │ │ +│ │ screenThread.Start() │ │ +│ │ │ │ +│ │ 4. InteractiveUIThread = new Thread(Flashing) │ │ +│ │ InteractiveUIThread.IsBackground = true │ │ +│ │ InteractiveUIThread.Start() │ │ +│ │ │ │ +│ │ 5. touchThread = new Thread(TouchLoop.Touch) │ │ +│ │ touchThread.IsBackground = true │ │ +│ │ touchThread.Start() │ │ +│ │ │ │ +│ │ 6. monitorThread = new Thread(MonitorPortsLoop) │ │ +│ │ monitorThread.IsBackground = true │ │ +│ │ monitorThread.Priority = ThreadPriority.Highest │ │ +│ │ monitorThread.Start() │ │ +│ └─────────────────────────────────────────────────────────┘ │ +└─────────────────────────────────────────────────────────────┘ +``` + +### Shutdown Sequence + +``` +Application Shutdown: +┌─────────────────────────────────────────────────────────────┐ +│ Window Closing Event │ +│ │ │ +│ ▼ │ +│ ┌─────────────────┐ │ +│ │ Set Shutdown │ │ +│ │ Flags │ │ +│ │ isRunning=false │ │ +│ │ serialThreadRun=│ │ +│ │ false │ │ +│ └─────────────────┘ │ +│ │ │ +│ ▼ │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ Cleanup Resources: │ │ +│ │ │ │ +│ │ 1. Close Serial Port │ │ +│ │ if (_port != null && _port.IsOpen) │ │ +│ │ _port.Close() │ │ +│ │ │ │ +│ │ 2. Dispose Timers │ │ +│ │ heatingTimer?.Dispose() │ │ +│ │ coolingTimer?.Dispose() │ │ +│ │ pouringTimer?.Dispose() │ │ +│ │ (etc.) │ │ +│ │ │ │ +│ │ 3. Wait for Background Threads │ │ +│ │ (Currently missing - threads may not exit cleanly) │ │ +│ │ │ │ +│ │ ⚠️ Issues: │ │ +│ │ • No proper thread termination synchronization │ │ +│ │ • Threads may continue running after main exit │ │ +│ │ • Resource cleanup may be incomplete │ │ +│ └─────────────────────────────────────────────────────────┘ │ +└─────────────────────────────────────────────────────────────┘ +``` + +## Performance Considerations + +### Thread Performance Profile + +| Thread | CPU Usage | Memory Impact | I/O Operations | Critical Level | +|--------|-----------|---------------|----------------|----------------| +| MonitorPortsLoop | High (continuous) | Medium | Serial I/O | Critical | +| SerialThreadLoop | Medium (event-driven) | Low | High Serial I/O | High | +| ScreenLoop | Low (10ms intervals) | Low | UI Updates | Medium | +| InteractiveUILoop | Low (animations) | Low | UI Updates | Low | +| TouchLoop | Very Low | Very Low | Event checks | Low | +| CheckInternetLoop | Low (periodic) | Low | Network I/O | Low | + +### Optimization Opportunities + +``` +Performance Improvements: +┌─────────────────────────────────────────────────────────────┐ +│ OPTIMIZATION TARGETS │ +│ │ +│ 1. MonitorPortsLoop Optimization: │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ Current: Continuous loop with complex logic │ │ +│ │ Improved: Event-driven state machine │ │ +│ │ │ │ +│ │ • Reduce CPU usage by 50-70% │ │ +│ │ • Eliminate unnecessary temperature checks │ │ +│ │ • Implement smart polling intervals │ │ +│ │ • Cache frequently accessed data │ │ +│ └─────────────────────────────────────────────────────────┘ │ +│ │ +│ 2. UI Thread Efficiency: │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ Current: Frequent UI updates from multiple threads │ │ +│ │ Improved: Batched updates with change detection │ │ +│ │ │ │ +│ │ • Reduce UI update frequency │ │ +│ │ • Implement dirty flag pattern │ │ +│ │ • Batch related updates together │ │ +│ │ • Use data binding for automatic updates │ │ +│ └─────────────────────────────────────────────────────────┘ │ +│ │ +│ 3. Memory Usage Optimization: │ +│ ┌─────────────────────────────────────────────────────────┐ │ +│ │ Current: Potential memory leaks from timers/events │ │ +│ │ Improved: Proper resource management │ │ +│ │ │ │ +│ │ • Implement IDisposable pattern │ │ +│ │ • Use weak event patterns │ │ +│ │ • Pool frequently allocated objects │ │ +│ │ • Monitor memory usage with metrics │ │ +│ └─────────────────────────────────────────────────────────┘ │ +└─────────────────────────────────────────────────────────────┘ +``` + +This threading and concurrency specification highlights the complex multi-threaded nature of the current system and the significant concurrency issues that need to be addressed in any migration to ensure system reliability and maintainability.