feat: [#164] implement ReentrantMutex<RefCell<UserOutput>> pattern

Cargo.toml

@@ -47,6 +47,7 @@ chrono = { version = "0.4", features = [ "serde" ] }
 clap = { version = "4.0", features = [ "derive" ] }
 derive_more = "0.99"
 figment = { version = "0.10", features = [ "json" ] }
+parking_lot = "0.12"
 rust-embed = "8.0"
 serde = { version = "1.0", features = [ "derive" ] }
 serde_json = "1.0"

docs/decisions/README.md

@@ -4,23 +4,24 @@ This directory contains architectural decision records for the Torrust Tracker D
 
 ## Decision Index
 
-| Status      | Date       | Decision                                                                                            | Summary                                                                                   |
-| ----------- | ---------- | --------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------- |
-| ✅ Accepted | 2025-11-11 | [Remove UserOutput Mutex](./user-output-mutex-removal.md)                                           | Remove Arc<Mutex<UserOutput>> pattern for simplified, deadlock-free architecture          |
-| ✅ Accepted | 2025-11-07 | [ExecutionContext Wrapper Pattern](./execution-context-wrapper.md)                                  | Use ExecutionContext wrapper around Container for future-proof command signatures         |
-| ✅ Accepted | 2025-11-03 | [Environment Variable Prefix](./environment-variable-prefix.md)                                     | Use `TORRUST_TD_` prefix for all environment variables                                    |
-| ✅ Accepted | 2025-10-15 | [External Tool Adapters Organization](./external-tool-adapters-organization.md)                     | Consolidate external tool wrappers in `src/adapters/` for better discoverability          |
-| ✅ Accepted | 2025-10-10 | [Repository Rename to Deployer](./repository-rename-to-deployer.md)                                 | Rename from "Torrust Tracker Deploy" to "Torrust Tracker Deployer" for production use     |
-| ✅ Accepted | 2025-10-03 | [Error Context Strategy](./error-context-strategy.md)                                               | Use structured error context with trace files for complete error information              |
-| ✅ Accepted | 2025-10-03 | [Command State Return Pattern](./command-state-return-pattern.md)                                   | Commands return typed states (Environment<S> → Environment<T>) for compile-time safety    |
-| ✅ Accepted | 2025-10-03 | [Actionable Error Messages](./actionable-error-messages.md)                                         | Use tiered help system with brief tips + .help() method for detailed troubleshooting      |
-| ✅ Accepted | 2025-10-01 | [Type Erasure for Environment States](./type-erasure-for-environment-states.md)                     | Use enum-based type erasure to enable runtime handling and serialization of typed states  |
-| ✅ Accepted | 2025-09-29 | [Test Context vs Deployment Environment Naming](./test-context-vs-deployment-environment-naming.md) | Rename TestEnvironment to TestContext to avoid conflicts with multi-environment feature   |
-| ✅ Accepted | 2025-09-10 | [LXD VMs over Containers](./lxd-vm-over-containers.md)                                              | Use LXD virtual machines instead of containers for production alignment                   |
-| ✅ Accepted | 2025-09-09 | [Tera Minimal Templating Strategy](./tera-minimal-templating-strategy.md)                           | Use Tera with minimal variables and templates to avoid complexity and delimiter conflicts |
-| ✅ Accepted | -          | [LXD over Multipass](./lxd-over-multipass.md)                                                       | Choose LXD containers over Multipass VMs for deployment testing                           |
-| ✅ Resolved | -          | [Docker Testing Evolution](./docker-testing-evolution.md)                                           | Evolution from Docker rejection to hybrid approach for split E2E testing                  |
-| ✅ Accepted | -          | [Meson Removal](./meson-removal.md)                                                                 | Remove Meson build system from the project                                                |
+| Status        | Date       | Decision                                                                                            | Summary                                                                                   |
+| ------------- | ---------- | --------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------- |
+| ✅ Accepted   | 2025-11-11 | [Use ReentrantMutex Pattern for UserOutput Reentrancy](./reentrant-mutex-useroutput-pattern.md)     | Use Arc<ReentrantMutex<RefCell<UserOutput>>> to fix same-thread deadlock in issue #164    |
+| ❌ Superseded | 2025-11-11 | [Remove UserOutput Mutex](./user-output-mutex-removal.md)                                           | Remove Arc<Mutex<UserOutput>> pattern for simplified, deadlock-free architecture          |
+| ✅ Accepted   | 2025-11-07 | [ExecutionContext Wrapper Pattern](./execution-context-wrapper.md)                                  | Use ExecutionContext wrapper around Container for future-proof command signatures         |
+| ✅ Accepted   | 2025-11-03 | [Environment Variable Prefix](./environment-variable-prefix.md)                                     | Use `TORRUST_TD_` prefix for all environment variables                                    |
+| ✅ Accepted   | 2025-10-15 | [External Tool Adapters Organization](./external-tool-adapters-organization.md)                     | Consolidate external tool wrappers in `src/adapters/` for better discoverability          |
+| ✅ Accepted   | 2025-10-10 | [Repository Rename to Deployer](./repository-rename-to-deployer.md)                                 | Rename from "Torrust Tracker Deploy" to "Torrust Tracker Deployer" for production use     |
+| ✅ Accepted   | 2025-10-03 | [Error Context Strategy](./error-context-strategy.md)                                               | Use structured error context with trace files for complete error information              |
+| ✅ Accepted   | 2025-10-03 | [Command State Return Pattern](./command-state-return-pattern.md)                                   | Commands return typed states (Environment<S> → Environment<T>) for compile-time safety    |
+| ✅ Accepted   | 2025-10-03 | [Actionable Error Messages](./actionable-error-messages.md)                                         | Use tiered help system with brief tips + .help() method for detailed troubleshooting      |
+| ✅ Accepted   | 2025-10-01 | [Type Erasure for Environment States](./type-erasure-for-environment-states.md)                     | Use enum-based type erasure to enable runtime handling and serialization of typed states  |
+| ✅ Accepted   | 2025-09-29 | [Test Context vs Deployment Environment Naming](./test-context-vs-deployment-environment-naming.md) | Rename TestEnvironment to TestContext to avoid conflicts with multi-environment feature   |
+| ✅ Accepted   | 2025-09-10 | [LXD VMs over Containers](./lxd-vm-over-containers.md)                                              | Use LXD virtual machines instead of containers for production alignment                   |
+| ✅ Accepted   | 2025-09-09 | [Tera Minimal Templating Strategy](./tera-minimal-templating-strategy.md)                           | Use Tera with minimal variables and templates to avoid complexity and delimiter conflicts |
+| ✅ Accepted   | -          | [LXD over Multipass](./lxd-over-multipass.md)                                                       | Choose LXD containers over Multipass VMs for deployment testing                           |
+| ✅ Resolved   | -          | [Docker Testing Evolution](./docker-testing-evolution.md)                                           | Evolution from Docker rejection to hybrid approach for split E2E testing                  |
+| ✅ Accepted   | -          | [Meson Removal](./meson-removal.md)                                                                 | Remove Meson build system from the project                                                |
 
 ## ADR Template
 

docs/decisions/reentrant-mutex-useroutput-pattern.md

@@ -0,0 +1,224 @@
+# Decision: Use ReentrantMutex Pattern for UserOutput Reentrancy
+
+## Status
+
+Accepted
+
+## Date
+
+2025-11-11
+
+## Context
+
+The Torrust Tracker Deployer application was experiencing reentrancy deadlocks with `Arc<std::sync::Mutex<UserOutput>>` when the same thread attempted to acquire the UserOutput lock multiple times. This occurred in scenarios where:
+
+### The Deadlock Problem
+
+In `CreateTemplateCommandController`, we encountered a critical deadlock where:
+
+1. **Controller Level**: `display_success_and_guidance()` acquired the `UserOutput` mutex for error handling
+2. **Progress Reporting**: While holding the lock, it called `self.progress.complete()`
+3. **Same Thread Deadlock**: `ProgressReporter.complete()` tried to acquire the same mutex on the same thread
+4. **Result**: Tests hung indefinitely requiring timeout mechanisms
+
+### Problem Pattern
+
+```rust
+// Deadlock scenario with std::sync::Mutex
+let user_output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Info)));
+
+// First acquisition in controller
+let lock1 = user_output.lock().unwrap();
+// ... controller work that calls ProgressReporter ...
+
+// Second acquisition in ProgressReporter (same thread)
+let lock2 = user_output.lock().unwrap(); // ❌ DEADLOCK!
+```
+
+This pattern was particularly problematic because:
+
+- Error handling and progress reporting are legitimate concurrent concerns
+- Both need access to UserOutput for displaying messages to users
+- The call stack naturally leads from controllers → progress reporters → user output
+- Standard Rust mutexes are not reentrant
+
+### GitHub Issue
+
+This decision addresses [GitHub Issue #164](https://github.com/torrust/torrust-tracker-deployer/issues/164): "Eliminate potential deadlock with Arc<Mutex<UserOutput>> reentrancy"
+
+## Decision
+
+**Replace `Arc<std::sync::Mutex<UserOutput>>` with `Arc<parking_lot::ReentrantMutex<RefCell<UserOutput>>>` throughout the codebase.**
+
+### Pattern Components
+
+1. **`parking_lot::ReentrantMutex`**: Allows the same thread to acquire the lock multiple times safely
+2. **`RefCell`**: Provides interior mutability since `ReentrantMutex<T>` only gives `&T` but UserOutput methods need `&mut self`
+3. **`Arc`**: Maintains shared ownership across components
+
+### New Pattern
+
+```rust
+use std::cell::RefCell;
+use std::sync::Arc;
+use parking_lot::ReentrantMutex;
+
+// New pattern that eliminates deadlocks
+let user_output = Arc::new(ReentrantMutex::new(RefCell::new(
+    UserOutput::new(VerbosityLevel::Info)
+)));
+
+// First acquisition in controller
+let lock1 = user_output.lock();
+{
+    let mut output1 = lock1.borrow_mut();
+    output1.info("Controller processing...");
+}
+
+// Second acquisition in ProgressReporter (same thread) - NO DEADLOCK!
+let lock2 = user_output.lock(); // ✅ Works correctly
+{
+    let mut output2 = lock2.borrow_mut();
+    output2.progress("50% complete");
+}
+```
+
+### Usage Pattern
+
+```rust
+// Standard usage in components
+let lock = user_output.lock();
+{
+    let mut output = lock.borrow_mut();
+    output.success("Operation completed");
+    output.data(&format!("Details: {}", details));
+} // RefCell borrow dropped here
+// ReentrantMutex lock can be held longer if needed
+```
+
+## Consequences
+
+### Positive
+
+- **Eliminates Deadlocks**: Same-thread reentrancy is now safe and supported
+- **Maintains Thread Safety**: Still protects UserOutput from concurrent access between different threads
+- **Preserves API**: UserOutput methods continue to work with `&mut self` through RefCell
+- **Clear Intent**: The type signature documents that reentrancy is expected and supported
+- **Performance**: `parking_lot::ReentrantMutex` has similar performance characteristics to `std::sync::Mutex`
+- **Minimal Changes**: UserOutput API remains unchanged, only synchronization mechanism updated
+
+### Negative
+
+- **Additional Dependency**: Introduces dependency on `parking_lot` crate
+- **Runtime Borrow Checking**: RefCell uses runtime borrow checking instead of compile-time (though panics are highly unlikely in our usage pattern)
+- **Slightly More Complex**: The type signature is more complex: `Arc<ReentrantMutex<RefCell<UserOutput>>>` vs `Arc<Mutex<UserOutput>>`
+- **Learning Curve**: Developers need to understand the ReentrantMutex + RefCell pattern
+
+### Migration Impact
+
+- **Components Updated**: Container, ExecutionContext, all controllers, ProgressReporter
+- **API Compatibility**: No changes to UserOutput public API - all methods continue to work identically
+- **Test Verification**: Integration tests verify the deadlock scenario is resolved
+
+### Thread Safety Limitations
+
+The current solution using `RefCell` has important thread safety limitations:
+
+- **Single-Thread Only**: `RefCell` does not implement `Sync`, making the pattern unsuitable for multi-threaded access
+- **Same-Thread Reentrancy**: The solution only addresses reentrancy within a single thread, not concurrent access from multiple threads
+- **Current Use Case**: This limitation is acceptable for the current codebase, which accesses `UserOutput` from a single thread during command execution
+
+**Multi-Threading Alternative**: If future requirements need multi-threaded access to `UserOutput`, consider:
+
+```rust
+Arc<ReentrantMutex<Arc<Mutex<UserOutput>>>>
+```
+
+However, this would reintroduce the original deadlock problem and require a different architectural solution (e.g., message passing or separate UserOutput instances per thread).
+
+## Alternatives Considered
+
+### Option 1: Arc<RwLock<UserOutput>> (Rejected)
+
+**Pros:**
+
+- Standard library solution
+- Multiple readers possible
+
+**Cons:**
+
+- Still not reentrant - would deadlock on write → write from same thread
+- UserOutput methods need `&mut self`, so read access isn't useful
+- More complex than needed for single-writer use case
+
+### Option 2: Remove Mutex Entirely (Considered in Previous ADR)
+
+**Pros:**
+
+- Completely eliminates synchronization complexity
+- Simple ownership patterns
+
+**Cons:**
+
+- Loses thread safety guarantees
+- Requires extensive architectural changes
+- Breaks shared UserOutput patterns needed for progress reporting
+- Would require passing UserOutput explicitly through all layers
+
+**Status**: This approach was documented in a previous ADR but ultimately rejected in favor of the ReentrantMutex solution.
+
+### Option 3: Message-Passing Architecture (Rejected)
+
+**Pros:**
+
+- Completely eliminates shared mutable state
+- Natural async boundaries
+
+**Cons:**
+
+- Massive architectural change requiring redesign of entire presentation layer
+- Overkill for this specific reentrancy issue
+- Would require rewriting all UserOutput usage patterns
+- Significantly more complex than the focused ReentrantMutex solution
+
+## Error Handling
+
+RefCell will panic if:
+
+- Attempting to borrow mutably while already borrowed mutably (same thread)
+- Attempting to borrow mutably while borrowed immutably
+
+In our usage pattern, this is extremely unlikely because:
+
+- We only use mutable borrows (never immutable)
+- Borrows are immediately dropped after UserOutput method calls
+- No long-lived borrows across function calls
+
+## Testing Strategy
+
+The solution includes integration tests that verify:
+
+1. Same-thread multiple acquisitions work without deadlock
+2. RefCell interior mutability enables `&mut self` methods
+3. Production code patterns work correctly
+
+See `tests/user_output_reentrancy.rs` for complete test verification.
+
+## Related Decisions
+
+This decision **supersedes**:
+
+- [Remove UserOutput Mutex for Simplified Architecture](./user-output-mutex-removal.md) - Marked as superseded by this ReentrantMutex approach
+
+This decision enables:
+
+- Safer progress reporting patterns
+- More natural error handling in nested call stacks
+- Future expansion of UserOutput usage without deadlock concerns
+
+## References
+
+- [GitHub Issue #164](https://github.com/torrust/torrust-tracker-deployer/issues/164)
+- [parking_lot ReentrantMutex Documentation](https://docs.rs/parking_lot/latest/parking_lot/struct.ReentrantMutex.html)
+- [RefCell Documentation](https://doc.rust-lang.org/std/cell/struct.RefCell.html)
+- [Integration Test: tests/user_output_reentrancy.rs](../../tests/user_output_reentrancy.rs)

docs/decisions/user-output-mutex-removal.md

@@ -2,7 +2,7 @@
 
 ## Status
 
-Accepted
+**Superseded** by [Use ReentrantMutex Pattern for UserOutput Reentrancy](./reentrant-mutex-useroutput-pattern.md)
 
 ## Date
 
@@ -12,41 +12,7 @@ Accepted
 
 The current `UserOutput` implementation uses `Arc<Mutex<UserOutput>>` throughout the codebase to prevent mixed output during operations. However, this approach has introduced several significant problems:
 
-### Deadlock Issues
-
-We recently encountered a critical reentrancy deadlock in `CreateTemplateCommandController` where:
-
-1. `display_success_and_guidance()` acquired the `UserOutput` mutex
-2. While holding the lock, it called `self.progress.complete()`
-3. `ProgressReporter.complete()` attempted to acquire the same mutex again
-4. **Deadlock**: Tests hung indefinitely requiring timeout mechanisms
-
-This pattern is fragile and could easily recur as the codebase grows, especially when using `UserOutput` through multiple wrapper types (like `ProgressReporter`).
-
-### Application Characteristics
-
-The Torrust Tracker Deployer has specific characteristics that make complex concurrency unnecessary:
-
-- **Single-user tool**: Intended for individual developers deploying on their machines
-- **Short execution time**: Total deployment time < 5 minutes
-- **Sequential operations**: Most steps must execute sequentially (provision → configure → deploy)
-- **Error recovery priority**: Clear error states more important than minor performance gains
-
-### Current Complexity vs Benefits
-
-The mutex introduces significant complexity:
-
-- Timeout mechanisms needed to prevent deadlocks
-- Complex error handling for mutex poisoning
-- Difficult debugging when deadlocks occur
-- `Arc<Mutex<>>` patterns throughout the codebase
-- Multiple access paths to the same mutex (direct + through wrappers)
-
-The benefits (preventing mixed output) don't justify this complexity for a sequential, single-user deployment tool.
-
-## Decision
-
-**Remove the `Arc<Mutex<UserOutput>>` pattern entirely and use direct ownership/borrowing of `UserOutput`.**
+**Note**: This ADR was superseded by the ReentrantMutex pattern approach, which solves the same deadlock issues while maintaining thread safety and requiring minimal architectural changes.
 
 ### Core Changes
 

project-words.txt

@@ -218,6 +218,7 @@ usermod
 userspace
 usize
 utmp
+useroutput
 vbqajnc
 viewmodel
 webservers