Skip to content

fix: use std::lock_guard in v8js_ini_string to prevent double-unlock - #549

Open
Appla wants to merge 1 commit into
phpv8:php8from
Appla:fix-lock-issue-v1
Open

Appla wants to merge 1 commit into
phpv8:php8from
Appla:fix-lock-issue-v1

Conversation

@Appla

@Appla Appla commented Sep 16, 2026

Copy link
Copy Markdown

I discovered a double‑unlock issue in v8js_ini_string while implementing v8js.thread_pool_size. The function manually called lock() and unlock(), which could result in the mutex being unlocked twice under certain conditions.

This patch replaces the manual locking with std::lock_guard<std::mutex>, ensuring the mutex is released exactly once and eliminating the double‑unlock bug in ZTS builds.

Copilot AI lite review requested due to automatic review settings September 16, 2026 02:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved review issues remain.

Pull request overview

Fixes a ZTS double-unlock bug in v8js_ini_string by using RAII-based mutex locking.

Changes:

  • Replaces manual locking with std::lock_guard<std::mutex>.
  • Preserves empty-value handling.
File summaries
File Summary
v8js_main.cc Safely updates INI strings with mutex protection.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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