Skip to content

Enable CHECK_REF_COUNTED_LIFECYCLE in WebKit - #25705

Merged
Jarred-Sumner merged 3 commits into
mainfrom
enable-ref-counted-lifecycle
Dec 27, 2025
Merged

Jarred-Sumner merged 3 commits into
mainfrom
enable-ref-counted-lifecycle

Conversation

@sosukesuzuki

@sosukesuzuki sosukesuzuki commented Dec 26, 2025 •

Copy link
Copy Markdown
Member

What does this PR do?

Enables CHECK_REF_COUNTED_LIFECYCLE in WebKit ( oven-sh/WebKit#121 )

See also WebKit/WebKit@a978fae

CHECK_REF_COUNTED_LIFECYCLE?

A compile-time macro that enables lifecycle validation for reference-counted objects in debug builds.

Definition

  #if ASSERT_ENABLED || ENABLE(SECURITY_ASSERTIONS)
  #define CHECK_REF_COUNTED_LIFECYCLE 1
  #else
  #define CHECK_REF_COUNTED_LIFECYCLE 0
  #endif

Purpose

Detects three categories of bugs:

  1. Missing adoption - Objects stored in RefPtr without using adoptRef()
  2. Ref during destruction - ref() called while destructor is running (causes dangling pointers)
  3. Thread safety violations - Unsafe ref/deref across threads

Implementation

When enabled, RefCountDebugger adds two tracking flags:

  • m_deletionHasBegun - Set when destructor starts
  • m_adoptionIsRequired - Cleared when adoptRef() is called

These flags are checked on every ref()/deref() call, with assertions failing on violations.

Motivation

Refactored debug code into a separate RefCountDebugger class to:

  • Improve readability of core refcount logic
  • Eliminate duplicate code across RefCounted, ThreadSafeRefCounted, etc.
  • Simplify adding new refcount classes

Overhead

Zero in release builds - the flags and checks are compiled out entirely.

How did you verify your code works?

@robobun

robobun commented Dec 26, 2025 •

Copy link
Copy Markdown
Collaborator
Updated 12:47 AM PT - Dec 27th, 2025

❌ @sosukesuzuki, your commit 36a8179 has 3 failures in Build #33787 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 25705

That installs a local version of the PR into your bun-25705 executable, so you can run:

bun-25705 --bun

sosukesuzuki and others added 2 commits December 27, 2025 09:51
SerializedScriptValue inherits from ThreadSafeRefCounted, so it must be
freed via deref() rather than direct delete. When CHECK_REF_COUNTED_LIFECYCLE
is enabled, direct delete causes assertion failure because willDelete()
is never called.

The value was leaked via leakRef() which leaves refcount at 1, so calling
deref() will properly decrement to 0 and delete the object through the
correct RefCounted lifecycle.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@sosukesuzuki
sosukesuzuki marked this pull request as ready for review December 27, 2025 14:20
@coderabbitai

coderabbitai Bot commented Dec 27, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Updates the default WebKit version hash in the CMake configuration and modifies memory management in the serialization bindings. The WebKit version is bumped from one commit hash to another, and the memory deallocation mechanism is changed from direct deletion to reference counting.

Changes

Cohort / File(s) Summary
WebKit version configuration
cmake/tools/SetupWebKit.cmake
Updates default WEBKIT_VERSION hash from 2317e14... to 863778...
Serialization memory management
src/bun.js/bindings/Serialization.cpp
Changes deallocation in Bun__SerializedScriptSlice__free from delete value; to value->deref(); for proper reference-counted lifecycle handling

Possibly related PRs

Pre-merge checks

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive PR description covers main objective and includes code explanation, but verification section is empty. Complete the 'How did you verify your code works?' section with specific testing details or verification steps performed.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main objective of enabling CHECK_REF_COUNTED_LIFECYCLE in WebKit, which aligns with the changes made.

Comment @coderabbitai help to get the list of available commands and usage tips.

@Jarred-Sumner
Jarred-Sumner merged commit 603bbd1 into main Dec 27, 2025
53 of 55 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the enable-ref-counted-lifecycle branch December 27, 2025 23:02
This was referenced Jan 16, 2026
@robobun robobun mentioned this pull request Mar 18, 2026
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.

3 participants