php / php/php-src

Ref counting based object release is not applied if cycle is broken in destructor during GC run

Open
#13,683 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Bug Category: GC Status: Needs Triage
Dominant language
C
Stars
40.4k
Forks
8.1k
Avg merge
2d 13h
Merged PRs (30d)
96

Description

Description

repro: https://3v4l.org/USrWu

see UPDATE below

It seems the GC can be improved. Currently, when a cycle is broken during GC run, a 2nd run of GC is needed to really release the object.

https://github.com/php/php-src/commit/b58d74547f "solves this issue" since PHP 8.2+, but is that the most efficient solution for everything? If a cycle (all cycles/references to an object) is explicitly broken during GC, is there anything againts releasing that object by standard refcounting?

Resulted in this output:

d
gc
bool(true) <- notice object is kept alive after the destructor has broken the cycle (and refcount dropped to 0)
gc rerun
bool(false)
bool(false)
done

But I expected this output instead:

d
gc
bool(false)
gc rerun
bool(false)
bool(false)
done

UPDATE:

Based on https://3v4l.org/mgoqd I would call this a bug as 2nd WeakReference::get() call is expected to always give the same result if no GC is run in between the calls.

PHP Version

any (tested PHP 7.4, 8.3)

Operating System

any

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by running the reproduction at https://3v4l.org/USrWu and compare it with the updated example at https://3v4l.org/mgoqd, focusing on GC, destructors, reference counts, and WeakReference::get(). Done means a cycle broken during a GC run releases the object immediately, so both WeakReference checks match the expected output without a second GC run.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.