Skip to content

[REFACTOR] Fix misleading lockfree_queue naming - Kent Beck Reveals Intention #359

Description

@kcenon

Summary

Fix misleading lockfree_queue naming that violates Kent Beck's "Reveals Intention" principle. The class uses fine-grained locking, not lock-free algorithms.

Current State

File: include/kcenon/thread/lockfree/lockfree_queue.h

Current Implementation (lines 99-303)

namespace kcenon::thread {

/**
 * @class concurrent_queue
 * @brief Thread-safe MPMC queue with blocking wait support
 *
 * This class implements a thread-safe Multi-Producer Multi-Consumer (MPMC) queue
 * using fine-grained locking for simplicity and correctness.
 * ...
 */
template <typename T>
class concurrent_queue {
    // Uses mutex for synchronization
    mutable std::mutex head_mutex_;
    mutable std::mutex tail_mutex_;
    mutable std::mutex cv_mutex_;
    std::condition_variable cv_;
};

// Misleading alias
template <typename T>
using lockfree_queue [[deprecated("Use concurrent_queue instead. "
    "This class uses fine-grained locking, not lock-free algorithms.")]] = concurrent_queue<T>;

}  // namespace kcenon::thread

Problem Analysis

Issue Explanation
Name implies lock-free lockfree_queue suggests atomic-only operations
Actually uses mutexes Fine-grained locking with head/tail mutexes
User expectation mismatch Users may choose this for real-time requirements
Already deprecated Deprecation exists but alias still available

Kent Beck Principle Violated

"Reveals Intention": The name does not accurately describe the implementation.

Semantic Difference

Term Meaning This Class
Lock-free No mutex, uses CAS loops
Wait-free Guaranteed progress
Fine-grained locking Separate locks per resource
Concurrent Thread-safe

Proposed Solution

Phase 1: Complete deprecation (Current release)

// Already done - verify deprecation message is clear
template <typename T>
using lockfree_queue [[deprecated(
    "MISLEADING NAME: This class uses fine-grained locking, not lock-free algorithms. "
    "Use concurrent_queue instead. "
    "For true lock-free queue, see lockfree_job_queue with hazard pointers."
)]] = concurrent_queue<T>;

Phase 2: Move file (Next release)

# Before
include/kcenon/thread/lockfree/lockfree_queue.h

# After
include/kcenon/thread/concurrent/concurrent_queue.h

Phase 3: Update directory structure

include/kcenon/thread/
├── concurrent/                    # Renamed from lockfree/
│   └── concurrent_queue.h         # Fine-grained locking MPMC
├── lockfree/                      # True lock-free implementations only
│   └── lockfree_job_queue.h       # Uses hazard pointers (actual lock-free)
└── core/
    └── job_queue.h                # Mutex-based (unchanged)

Phase 4: Remove alias (Major version)

  • Remove lockfree_queue alias entirely
  • Update all internal usages

Tasks

  • Verify current deprecation message is clear
  • Update deprecation message to explain "why"
  • Create concurrent/ directory
  • Move concurrent_queue to new location
  • Update all internal includes
  • Add compatibility header at old location
  • Update documentation
  • Update examples

Acceptance Criteria

  • No class named lockfree_* unless truly lock-free
  • Clear documentation on what "concurrent" means
  • Smooth migration path with compatibility headers
  • All tests pass with new structure

Migration Guide for Users

// Before
#include <kcenon/thread/lockfree/lockfree_queue.h>
kcenon::thread::lockfree_queue<int> queue;  // Deprecated warning

// After
#include <kcenon/thread/concurrent/concurrent_queue.h>
kcenon::thread::concurrent_queue<int> queue;  // Correct

Related

Metadata

Metadata

Assignees

Labels

priority:mediumMedium priority issuerefactoringCode refactoring and improvements

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions