Skip to content

Fix race condition in PHPParserUnitTests caused by shared SQLite database#4032

Merged
Jarod42 merged 3 commits into
eranif:masterfrom
Manvikamboz:fix-php-parser-test-concurrency
Jul 3, 2026
Merged

Fix race condition in PHPParserUnitTests caused by shared SQLite database#4032
Jarod42 merged 3 commits into
eranif:masterfrom
Manvikamboz:fix-php-parser-test-concurrency

Conversation

@Manvikamboz

Copy link
Copy Markdown
Contributor

#4017 issue

This PR fixes intermittent failures in PHPParserUnitTests by isolating the SQLite database used by each test process.

Root Cause

The test suite registers individual test cases using doctest_discover_tests(), allowing CTest to execute them concurrently.

All test processes were using the same SQLite database:

/tmp/.codelite/phpsymbols.db

Since each test clears and repopulates the database, concurrent execution could result in:

  • One process deleting data created by another.
  • SQLite lock contention (SQLITE_BUSY).
  • Non-deterministic assertion failures due to missing symbols.

This matches the inconsistent failures reported across different build environments.

Changes

  • Replace the shared database filename with a process-unique database filename using wxGetProcessId().
  • Ensure each test process operates on its own isolated SQLite database.
  • Add cleanup to close the database connection and remove the temporary database after the tests complete.

Result

Tests no longer share mutable state during parallel execution, eliminating race conditions while preserving the existing parser behavior. The change only affects the test infrastructure and does not modify the parser implementation or production code.

Comment thread codelitephp/PHPParserUnitTests/main.cpp Outdated
@Manvikamboz

Copy link
Copy Markdown
Contributor Author

Thanks @Jarod42 for the suggestion! I updated the implementation to use an in-memory SQLite database (":memory:") instead of a process-specific temporary file.

To support this, I also updated PHPLookupTable::Open() to handle the ":memory:" database path by bypassing filesystem-specific checks and directory creation. The unit tests now use lookup.Open(":memory:"), and the temporary file creation/cleanup logic has been removed.

I've pushed the changes to this PR.

@Jarod42

Jarod42 commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Can you check if the tests marked as #if 1 // FAILED/doctest::may_fail() (as "test_phpdoc_var_in_class") now pass with that fix?
Thanks to activate them if they are fixed now.

@Manvikamboz

Copy link
Copy Markdown
Contributor Author

I incounter this issue the MSYS2 initialization crash with exit code 2147483652 (0x80000004 / STATUS_SINGLE_STEP) because of the error occur by :

  1. Emulation Crash (STATUS_SINGLE_STEP)
  2. Missing Pre-installed MSYS2
  3. Cache Synchronization

so I have modified msys2.yml to address this:

  1. Dynamic Installation Path
  2. Disabled Caching for ARM64
  3. Robust Environment Resolution

Comment thread .github/workflows/msys2.yml
@Jarod42 Jarod42 merged commit 3f81827 into eranif:master Jul 3, 2026
8 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.

2 participants