Skip to content

Commit 49e5c18

Browse files
rxduclaude
andcommitted
[Refactor] Production readiness: thread-safety, performance, and testing
- Add ASAN, UBSAN, TSAN sanitizer options to CMakeLists.txt - Add sanitizer CI jobs (ASAN+UBSAN, TSAN separately - mutually exclusive) - Fix null pointer dereference in A* multi-goal search (UBSAN detected) - Added is_multi_goal_search_ flag to SearchContext - A* strategy now checks flag before dereferencing goal iterator - Update README.md with production features section - Create CHANGELOG.md with unreleased changes - Update test count to 317 in README 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent ea503cc commit 49e5c18

7 files changed

Lines changed: 186 additions & 8 deletions

File tree

.github/workflows/ci.yml

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,42 @@ jobs:
9696
- uses: codecov/codecov-action@v5
9797
if: github.ref == 'refs/heads/main'
9898
with:
99-
fail_ci_if_error: true
99+
fail_ci_if_error: true
100100
files: ./build/coverage.info
101101
exclude: tests, src/demo
102102
flags: unittests # optional
103103
token: ${{ secrets.CODECOV_TOKEN }} # required
104-
verbose: true # optional (default = false)
104+
verbose: true # optional (default = false)
105+
106+
# AddressSanitizer + UndefinedBehaviorSanitizer
107+
sanitize-asan:
108+
runs-on: ubuntu-24.04
109+
steps:
110+
- uses: actions/checkout@v4
111+
- name: Update submodules
112+
run: git submodule update --init --recursive
113+
- name: Configure with ASAN + UBSAN
114+
run: mkdir build && cd build && cmake -DBUILD_TESTING=ON -DENABLE_ASAN=ON -DENABLE_UBSAN=ON -DCMAKE_BUILD_TYPE=Debug ..
115+
- name: Build
116+
run: cmake --build build
117+
- name: Run tests with ASAN + UBSAN
118+
env:
119+
ASAN_OPTIONS: detect_leaks=1:abort_on_error=1
120+
UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1
121+
run: cd build && ./bin/utests
122+
123+
# ThreadSanitizer (separate job - incompatible with ASAN)
124+
sanitize-tsan:
125+
runs-on: ubuntu-24.04
126+
steps:
127+
- uses: actions/checkout@v4
128+
- name: Update submodules
129+
run: git submodule update --init --recursive
130+
- name: Configure with TSAN
131+
run: mkdir build && cd build && cmake -DBUILD_TESTING=ON -DENABLE_TSAN=ON -DCMAKE_BUILD_TYPE=Debug ..
132+
- name: Build
133+
run: cmake --build build
134+
- name: Run tests with TSAN
135+
env:
136+
TSAN_OPTIONS: halt_on_error=1:second_deadlock_stack=1
137+
run: cd build && ./bin/utests

CHANGELOG.md

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# Changelog
2+
3+
All notable changes to this project will be documented in this file.
4+
5+
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
6+
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
7+
8+
## [Unreleased]
9+
10+
### Added
11+
- **PathResult struct**: Rich search results with `total_cost`, `nodes_expanded`, and `found` flag
12+
- **SearchLimits struct**: Early termination via `max_expansions` or `timeout_ms`
13+
- **Multi-goal search**: `Dijkstra::SearchMultiGoal()` and `AStar::SearchMultiGoal()` find path to nearest goal
14+
- **Nodes expanded counter**: `context.GetNodesExpanded()` for search diagnostics
15+
- **Debug cost assertions**: Validate non-negative, finite costs in debug builds (`debug_checks.hpp`)
16+
- **Production readiness documentation**: `docs/PRODUCTION_READINESS.md`
17+
- **14 new unit tests** for production features (317 total tests)
18+
19+
### Changed
20+
- **Deterministic tie-breaking**: Priority queue comparators now break ties by `vertex_id` for reproducible results
21+
22+
### Fixed
23+
- **Iterator invalidation**: `Edge` and `Vertex::vertices_from` now use stable `Vertex*` pointers instead of iterators that could invalidate on `unordered_map` rehash
24+
25+
## [3.0.1] - 2025-08-19
26+
27+
### Fixed
28+
- Codecov configuration updates
29+
30+
## [3.0.0] - 2025-08-19
31+
32+
### Added
33+
- Unified search framework with CRTP strategy pattern
34+
- Thread-safe concurrent searches via external `SearchContext`
35+
- Generic cost type support with `CostTraits`
36+
- `DynamicPriorityQueue` for efficient priority updates
37+
- Comprehensive exception handling with custom exception types
38+
- Structure validation for graphs and trees
39+
- Extensive documentation and tutorials
40+
41+
### Changed
42+
- Major API redesign for thread safety and generic costs
43+
- Search algorithms now accept `SearchContext&` for thread-safe operation
44+
- Improved memory management with RAII patterns
45+
46+
## [2.x] - Previous Releases
47+
48+
See git history for earlier changes.

CMakeLists.txt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ option(BUILD_TESTING "Build tests" OFF)
66
option(BUILD_SAMPLES "Build samples" ON)
77
option(STATIC_CHECK "Perform static check" OFF)
88
option(COVERAGE_CHECK "Perform coverage check" OFF)
9+
option(ENABLE_ASAN "Enable AddressSanitizer" OFF)
10+
option(ENABLE_UBSAN "Enable UndefinedBehaviorSanitizer" OFF)
11+
option(ENABLE_TSAN "Enable ThreadSanitizer" OFF)
912

1013
# sanity check of the options
1114
if (COVERAGE_CHECK)
@@ -32,6 +35,30 @@ if (COVERAGE_CHECK)
3235
endif ()
3336
endif ()
3437

38+
# Sanitizer configuration
39+
# Note: ASAN and TSAN are mutually exclusive
40+
if (ENABLE_ASAN AND ENABLE_TSAN)
41+
message(FATAL_ERROR "ASAN and TSAN cannot be enabled simultaneously")
42+
endif ()
43+
44+
if (ENABLE_ASAN)
45+
message(STATUS "AddressSanitizer enabled")
46+
add_compile_options(-fsanitize=address -fno-omit-frame-pointer -g)
47+
add_link_options(-fsanitize=address)
48+
endif ()
49+
50+
if (ENABLE_UBSAN)
51+
message(STATUS "UndefinedBehaviorSanitizer enabled")
52+
add_compile_options(-fsanitize=undefined -fno-omit-frame-pointer -g)
53+
add_link_options(-fsanitize=undefined)
54+
endif ()
55+
56+
if (ENABLE_TSAN)
57+
message(STATUS "ThreadSanitizer enabled")
58+
add_compile_options(-fsanitize=thread -fno-omit-frame-pointer -g)
59+
add_link_options(-fsanitize=thread)
60+
endif ()
61+
3562
## Additional cmake module path
3663
set(USER_CMAKE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
3764
list(APPEND CMAKE_MODULE_PATH "${USER_CMAKE_PATH}/modules")

README.md

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,29 @@ void worker_thread(const Graph<Location>* graph) {
8787
8888
**Graph modifications** require external synchronization (by design for performance).
8989
90+
### Production Features (v3.1.0)
91+
92+
**Rich search results** with cost and diagnostics:
93+
```cpp
94+
auto result = SearchAlgorithm<...>::SearchWithResult(graph, context, start, goal, strategy);
95+
if (result.found) {
96+
std::cout << "Cost: " << result.total_cost << ", Expanded: " << result.nodes_expanded;
97+
}
98+
```
99+
100+
**Bounded search** for real-time systems:
101+
```cpp
102+
auto limits = SearchLimits::MaxExpansions(1000); // or SearchLimits::Timeout(50)
103+
auto result = SearchAlgorithm<...>::SearchWithLimits(graph, context, start, goal, strategy, limits);
104+
```
105+
106+
**Multi-goal search** (find nearest goal):
107+
```cpp
108+
std::vector<Location> goals = {charging_station1, charging_station2, charging_station3};
109+
auto result = Dijkstra::SearchMultiGoal(graph, context, robot_position, goals);
110+
// result.goal_index tells which goal was reached
111+
```
112+
90113
---
91114
92115
## Build & Integration
@@ -144,7 +167,7 @@ cmake --build .
144167
./bin/simple_graph_demo
145168
./bin/thread_safe_search_demo
146169

147-
# Run comprehensive tests (199 tests, 100% pass rate)
170+
# Run comprehensive tests (317 tests, 100% pass rate)
148171
./bin/utests
149172
```
150173

@@ -186,6 +209,7 @@ auto path = Dijkstra::Search(map, {0, "Home"}, {1, "Work"});
186209
- **[Architecture Overview](docs/architecture.md)** - System design and template patterns
187210
- **[Advanced Features](docs/advanced_features.md)** - Custom costs, validation, thread safety
188211
- **[Search Algorithms Guide](docs/search_algorithms.md)** - Deep dive into A*, Dijkstra, BFS, DFS
212+
- **[Production Readiness](docs/PRODUCTION_READINESS.md)** - Robotics deployment guide
189213
190214
### **For Contributors**
191215
- **[Performance Testing](docs/performance_testing.md)** - Benchmarking and optimization

include/graph/search/astar.hpp

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,15 @@ class AStarStrategy : public SearchStrategy<AStarStrategy<State, Transition, Sta
6060

6161
void InitializeVertexImpl(SearchInfo& info, vertex_iterator vertex,
6262
vertex_iterator goal_vertex,
63-
typename Base::SearchContextType& /*context*/) const {
63+
typename Base::SearchContextType& context) const {
6464
info.SetGCost(Transition{});
65-
Transition h_cost = heuristic_(vertex->state, goal_vertex->state);
65+
66+
// In multi-goal mode, goal_vertex is vertex_end() (invalid), skip heuristic
67+
Transition h_cost{};
68+
if (!context.IsMultiGoalSearch()) {
69+
h_cost = heuristic_(vertex->state, goal_vertex->state);
70+
}
71+
6672
info.SetHCost(h_cost);
6773
info.SetFCost(Transition{} + h_cost);
6874
info.SetChecked(false);
@@ -73,7 +79,7 @@ class AStarStrategy : public SearchStrategy<AStarStrategy<State, Transition, Sta
7379
bool RelaxVertexImpl(SearchInfo& current_info, SearchInfo& successor_info,
7480
vertex_iterator successor_vertex, vertex_iterator goal_vertex,
7581
const Transition& edge_cost,
76-
typename Base::SearchContextType& /*context*/) const {
82+
typename Base::SearchContextType& context) const {
7783

7884
// Debug assertions for cost validity
7985
debug::AssertValidEdgeCost(edge_cost, "A* RelaxVertex: edge cost");
@@ -88,9 +94,13 @@ class AStarStrategy : public SearchStrategy<AStarStrategy<State, Transition, Sta
8894

8995
if (this->cost_comparator_(new_g_cost, successor_g_cost)) {
9096
successor_info.SetGCost(new_g_cost);
91-
Transition h_cost = heuristic_(successor_vertex->state, goal_vertex->state);
9297

93-
debug::AssertValidHeuristic(h_cost, "A* RelaxVertex: heuristic value");
98+
// In multi-goal mode, goal_vertex is vertex_end() (invalid), skip heuristic
99+
Transition h_cost{};
100+
if (!context.IsMultiGoalSearch()) {
101+
h_cost = heuristic_(successor_vertex->state, goal_vertex->state);
102+
debug::AssertValidHeuristic(h_cost, "A* RelaxVertex: heuristic value");
103+
}
94104

95105
successor_info.SetHCost(h_cost);
96106
successor_info.SetFCost(new_g_cost + h_cost);

include/graph/search/search_algorithm.hpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,9 @@ class SearchAlgorithm final {
331331
// Clear previous search data but preserve allocated memory
332332
context.Reset();
333333

334+
// Single-goal search mode
335+
context.SetMultiGoalSearch(false);
336+
334337
// Track start vertex for reliable path reconstruction
335338
context.SetStartVertexId(start->vertex_id);
336339

@@ -467,6 +470,9 @@ class SearchAlgorithm final {
467470
// Clear previous search data but preserve allocated memory
468471
context.Reset();
469472

473+
// Multi-goal search mode - strategies should not dereference goal_vertex
474+
context.SetMultiGoalSearch(true);
475+
470476
// Track start vertex for reliable path reconstruction
471477
context.SetStartVertexId(start->vertex_id);
472478

include/graph/search/search_context.hpp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,10 @@ class SearchContext {
399399
/// Useful for diagnostics and heuristic tuning
400400
size_t nodes_expanded_ = 0;
401401

402+
/// Flag indicating multi-goal search mode
403+
/// When true, strategies should not dereference goal_vertex (it may be vertex_end())
404+
bool is_multi_goal_search_ = false;
405+
402406
/// Default reserve size for moderate-sized graphs
403407
static constexpr size_t DEFAULT_RESERVE_SIZE = 1000;
404408

@@ -478,6 +482,30 @@ class SearchContext {
478482
nodes_expanded_ = 0;
479483
}
480484

485+
/**
486+
* @brief Check if this is a multi-goal search
487+
*
488+
* When true, strategies should not dereference goal_vertex iterators
489+
* as they may be vertex_end() (invalid). Use h_cost = 0 instead.
490+
*
491+
* @return True if multi-goal search mode is active
492+
*/
493+
bool IsMultiGoalSearch() const noexcept {
494+
return is_multi_goal_search_;
495+
}
496+
497+
/**
498+
* @brief Set multi-goal search mode
499+
*
500+
* Called by search algorithms to indicate multi-goal mode.
501+
* Strategies use this to avoid dereferencing invalid goal iterators.
502+
*
503+
* @param is_multi_goal True for multi-goal search, false for single-goal
504+
*/
505+
void SetMultiGoalSearch(bool is_multi_goal) noexcept {
506+
is_multi_goal_search_ = is_multi_goal;
507+
}
508+
481509
/**
482510
* @brief Get search information for a vertex
483511
* @param vertex_id The ID of the vertex
@@ -573,6 +601,8 @@ class SearchContext {
573601
start_vertex_id_ = -1;
574602
// Reset nodes expanded counter
575603
nodes_expanded_ = 0;
604+
// Reset multi-goal flag
605+
is_multi_goal_search_ = false;
576606
// Keep allocated memory in the map for next search
577607
// This avoids reallocating hash table buckets
578608
}

0 commit comments

Comments
 (0)