Fix/ttl segfault issue#18
Merged
Merged
Conversation
- Add ttl_config_tests module to detect segfaults - Test TTL configuration doesn't cause worker process crashes - Test TTL configuration merging from parent to child locations - Test multiple requests with TTL configuration - Add x402_ttl 60 to test configuration This addresses the segfault issue reported where worker processes exit with signal 11 (core dumped) after TTL configuration was added.
… pool The segfault was caused by directly copying ngx_str_t pointers from prev_conf to conf_mut in merge_loc_conf. When parent and child configurations use different memory pools, copying pointers can lead to dangling pointers that cause segfaults when accessed later. Fix: - Use copy_string_to_pool() to reallocate strings from the current configuration pool instead of copying pointers - Apply this fix to all string fields: amount_str, pay_to_str, facilitator_url_str, description_str, network_str, network_id_str, resource_str, asset_str, asset_decimals_str, timeout_str, facilitator_fallback_str, and ttl_str - Export common module to make copy_string_to_pool accessible This ensures all strings are allocated from the current configuration's memory pool, preventing segfaults when accessing configuration values during request processing.
Add comprehensive panic handling to prevent panics from crossing FFI boundaries, which would cause undefined behavior. This provides detailed logging when panics occur to help with debugging. Changes: - Create panic_handler module with panic protection utilities: - init_panic_hook(): Initialize global panic hook for FFI safety - catch_panic(): Catch panics and log detailed information - catch_panic_or_default(): Catch panics and return default value - catch_panic_with_info(): Catch panics with additional context - Wrap all FFI functions with panic protection: - x402_phase_handler: Protected with catch_panic_or_default - x402_ngx_handler: Protected with catch_panic_or_default - x402_metrics_handler: Protected with catch_panic_or_default - Auto-initialize panic hook in logging module - Add Safety documentation to copy_string_to_pool function Benefits: - Prevents undefined behavior from panics crossing FFI boundaries - Provides detailed panic logs including message, file location, and context - Helps debug issues in production by logging panics to nginx error log - All panics are caught and logged before they can crash nginx worker processes
Apply DRY principle to reduce code duplication in configuration merging. Changes: - Create merge_string_field! macro to handle string field merging - Replace 60 lines of repetitive code with 12 macro calls - Net reduction: 28 lines of code - Improve maintainability: changes to merge logic only need to be made once Benefits: - Follows DRY (Don't Repeat Yourself) principle - Easier to maintain: single point of change for merge logic - Easier to extend: adding new fields requires only one macro call - More readable: clear intent with concise syntax
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
1 participant
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.
No description provided.