Skip to content

C SDK: Make library thread-safe with mutex-protected allocator#51

Merged
clemensv merged 7 commits into
masterfrom
copilot/make-library-thread-safe
Dec 9, 2025
Merged

C SDK: Make library thread-safe with mutex-protected allocator#51
clemensv merged 7 commits into
masterfrom
copilot/make-library-thread-safe

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Dec 9, 2025

The C SDK had race conditions in global allocator access and was missing cleanup of the regex cache. Multiple threads calling validation functions concurrently could corrupt allocator state.

Changes

Thread-safe allocator access:

  • Protected g_allocator reads/writes with pthread_mutex_t (Unix) / CRITICAL_SECTION (Windows)
  • Used pthread_once / InitOnceExecuteOnce for race-free mutex initialization
  • Guarded js_malloc(), js_realloc(), js_free(), js_set_allocator(), js_get_allocator()

Cleanup lifecycle:

  • js_cleanup() now calls js_regex_cache_clear() to free compiled patterns
  • js_init() explicitly initializes allocator mutex (optional but recommended)

Documentation:

  • Added thread-safety semantics to types.h and json_structure.h
  • New "Thread Safety" section in README with usage guidelines
  • Documented that js_set_allocator() and js_cleanup() must not be called during validation

Testing:

  • Added 5 concurrent validation tests (4-8 threads, 50-1000 iterations)
  • Tests schema validation, instance validation, mixed operations, and init/cleanup lifecycle

Usage

// Initialize once before threading
js_init();

// Safe: concurrent validation from multiple threads
#pragma omp parallel for
for (int i = 0; i < 1000; i++) {
    js_result_t result;
    js_validate_instance(instance_json, schema_json, &result);
    js_result_cleanup(&result);
}

// Cleanup after all threads complete
js_cleanup();

Notes

  • Regex cache (already thread-safe with std::mutex) now properly cleaned up
  • One-time initialization via pthread_once persists for process lifetime
  • Library can be reinitialized after js_cleanup() but pthread_once state remains
Original prompt

This section details on the original issue you should resolve

<issue_title>C SDK: Make library fully thread-safe</issue_title>
<issue_description>## Problem

The C SDK has thread-safety issues with global state:

Global allocator (types.c:31-36):

  • g_allocator is mutable global state with no synchronization
  • js_set_allocator() modifies it without locking
  • js_malloc/js_realloc/js_free read it without locking

Initialization/cleanup:

  • js_init_with_allocator() and js_cleanup() modify global state
  • Calling these while validations are in progress causes undefined behavior
  • js_cleanup() doesn't clear the regex cache

The regex cache in regex_utils.cpp IS thread-safe (uses std::mutex).

Proposed Solution

  1. Protect allocator access:

    • Add mutex around g_allocator reads/writes
    • Or make allocator immutable after js_init()
    • Or use thread-local allocators
  2. Fix js_cleanup():

    • Call regex_cache_clear() to clean up regex cache
    • Document that cleanup must only be called when no validations are active
  3. Document thread-safety requirements:

    • js_init() must be called once before any threads start validation
    • js_set_allocator() must not be called during validation
    • js_cleanup() must be called after all validations complete

Tests to Add

  • Test concurrent validation from multiple pthreads
  • Test with ThreadSanitizer (TSan)
  • Verify no data races with concurrent js_validate_schema/js_validate_instance calls
  • Test init/cleanup lifecycle with proper ordering

Acceptance Criteria

  • Concurrent validation calls are safe (with proper init/cleanup)
  • ThreadSanitizer reports no races
  • js_cleanup() properly cleans up all resources
  • Thread safety documented in README and header comments</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits December 9, 2025 09:19
- Add mutex protection for g_allocator using pthread on Unix and CRITICAL_SECTION on Windows
- Protect js_set_allocator(), js_get_allocator(), js_malloc(), js_realloc(), js_free()
- Add js_init_allocator_mutex() and js_destroy_allocator_mutex() for lifecycle management
- Update js_init() to initialize mutex
- Update js_cleanup() to call js_regex_cache_clear() and destroy mutex
- Add comprehensive thread safety tests with concurrent validation
- Update documentation in headers and README with thread-safety guidelines
- Link pthread on Unix systems in CMakeLists.txt

Co-authored-by: clemensv <542030+clemensv@users.noreply.github.com>
- Replace manual initialization flag with pthread_once on Unix
- Use InitOnceExecuteOnce on Windows for thread-safe initialization
- This ensures no race condition even when multiple threads call allocator functions before js_init()
- Add comment about one-time initialization semantics in js_destroy_allocator_mutex()
- All tests still pass

Co-authored-by: clemensv <542030+clemensv@users.noreply.github.com>
- Clarify that js_cleanup() can be followed by js_init() for reinitialization
- Document that pthread_once/InitOnceExecuteOnce persist for process lifetime
- This addresses code review feedback about cleanup/reinit behavior

Co-authored-by: clemensv <542030+clemensv@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix thread safety issues in C SDK C SDK: Make library thread-safe with mutex-protected allocator Dec 9, 2025
Copilot AI requested a review from clemensv December 9, 2025 09:33
…ry allocation

The default malloc/free and realloc functions are already thread-safe on all
modern platforms. We only need mutex protection when changing the allocator
itself via js_set_allocator().

This removes the per-allocation locking overhead and fixes a potential issue
with CRITICAL_SECTION initialization on Windows.
@clemensv clemensv force-pushed the copilot/make-library-thread-safe branch from c696139 to cd34c81 Compare December 9, 2025 10:57
Clemens Vasters added 2 commits December 9, 2025 12:06
SRWLOCK has significant advantages over CRITICAL_SECTION:
- Zero-initialized by default (SRWLOCK_INIT = {0})
- No initialization function call needed
- No destruction needed
- Very lightweight and fast
- Available on Windows Vista and later

This simplifies the code and eliminates potential issues with
uninitialized or improperly destroyed CRITICAL_SECTION objects.
_beginthreadex returns handle (non-zero) on success and 0 on failure,
whereas pthread_create returns 0 on success and non-zero on failure.

Fixed by using a helper function that returns consistent error codes:
0 for success, non-zero for failure.
@clemensv clemensv marked this pull request as ready for review December 9, 2025 11:10
@clemensv clemensv merged commit fbdb590 into master Dec 9, 2025
11 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

C SDK: Make library fully thread-safe

2 participants