Skip to content

Commit 2c58248

Browse files
committed
Adjust GC threshold if num_roots higher than threshold after collection
This fixes an edge causing the GC to be triggered repeatedly. Destructors might add potential garbage to the buffer, so it may happen that num_root it higher than threshold after collection, thus triggering a GC run almost immediately. This can happen by touching enough objects in a destructor, e.g. by iterating over an array. If the happens again in the new run, and the threshold is not updated, the GC may be triggered repeatedly. Also prevent GC runs during zend_gc_check_root_tmpvars(), as the threshold is not adjusted yet a this point.
1 parent df72526 commit 2c58248

File tree

4 files changed

+181
-1
lines changed

4 files changed

+181
-1
lines changed

Zend/tests/gh13670_001.phpt

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
--TEST--
2+
GH-13670 001
3+
--FILE--
4+
<?php
5+
6+
register_shutdown_function(function () {
7+
global $shutdown;
8+
$shutdown = true;
9+
});
10+
11+
class Cycle {
12+
public $self;
13+
public function __construct() {
14+
$this->self = $this;
15+
}
16+
public function __destruct() {
17+
global $shutdown;
18+
if (!$shutdown) {
19+
new Cycle();
20+
}
21+
}
22+
}
23+
24+
$defaultThreshold = gc_status()['threshold'];
25+
for ($i = 0; $i < $defaultThreshold+1; $i++) {
26+
new Cycle();
27+
}
28+
29+
$objs = [];
30+
for ($i = 0; $i < 100; $i++) {
31+
$obj = new stdClass;
32+
$objs[] = $obj;
33+
}
34+
35+
$st = gc_status();
36+
37+
if ($st['runs'] > 10) {
38+
var_dump($st);
39+
}
40+
?>
41+
==DONE==
42+
--EXPECT--
43+
==DONE==

Zend/tests/gh13670_002.phpt

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
--TEST--
2+
GH-13670 002
3+
--FILE--
4+
<?php
5+
6+
register_shutdown_function(function () {
7+
global $shutdown;
8+
$shutdown = true;
9+
});
10+
11+
class Cycle {
12+
public $self;
13+
public function __construct() {
14+
$this->self = $this;
15+
}
16+
}
17+
18+
class Canary {
19+
public $self;
20+
public function __construct() {
21+
$this->self = $this;
22+
}
23+
public function __destruct() {
24+
global $shutdown;
25+
if (!$shutdown) {
26+
work();
27+
}
28+
}
29+
}
30+
31+
function work() {
32+
global $objs, $defaultThreshold;
33+
new Canary();
34+
// Create some collectable garbage so the next run will not adjust
35+
// threshold
36+
for ($i = 0; $i < 100; $i++) {
37+
new Cycle();
38+
}
39+
// Add potential garbage to buffer
40+
foreach (array_slice($objs, 0, $defaultThreshold) as $obj) {
41+
$o = $obj;
42+
}
43+
}
44+
45+
$defaultThreshold = gc_status()['threshold'];
46+
$objs = [];
47+
for ($i = 0; $i < $defaultThreshold*2; $i++) {
48+
$obj = new stdClass;
49+
$objs[] = $obj;
50+
}
51+
52+
work();
53+
54+
foreach ($objs as $obj) {
55+
$o = $obj;
56+
}
57+
58+
$st = gc_status();
59+
60+
if ($st['runs'] > 10) {
61+
var_dump($st);
62+
}
63+
?>
64+
==DONE==
65+
--EXPECT--
66+
==DONE==

Zend/tests/gh13670_003.phpt

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
--TEST--
2+
GH-13670 003
3+
--FILE--
4+
<?php
5+
6+
register_shutdown_function(function () {
7+
global $shutdown;
8+
$shutdown = true;
9+
});
10+
11+
class Cycle {
12+
public $self;
13+
public function __construct() {
14+
$this->self = $this;
15+
}
16+
}
17+
18+
class Canary {
19+
public $self;
20+
public function __construct() {
21+
$this->self = $this;
22+
}
23+
public function __destruct() {
24+
global $shutdown;
25+
if (!$shutdown) {
26+
work();
27+
}
28+
}
29+
}
30+
31+
function work() {
32+
global $objs, $defaultThreshold;
33+
new Canary();
34+
// Create some collectable garbage so the next run will not adjust
35+
// threshold
36+
for ($i = 0; $i < 100; $i++) {
37+
new Cycle();
38+
}
39+
// Add potential garbage to buffer
40+
foreach (array_slice($objs, 0, $defaultThreshold) as $obj) {
41+
$o = $obj;
42+
}
43+
}
44+
45+
$defaultThreshold = gc_status()['threshold'];
46+
$objs = [];
47+
for ($i = 0; $i < $defaultThreshold*2; $i++) {
48+
$obj = new stdClass;
49+
$objs[] = $obj;
50+
}
51+
52+
work();
53+
54+
// Iterate on a tmpvar
55+
foreach (array_slice($objs, -10) as $obj) {
56+
$o = $obj;
57+
}
58+
59+
$st = gc_status();
60+
61+
if ($st['runs'] > 10) {
62+
var_dump($st);
63+
}
64+
?>
65+
==DONE==
66+
--EXPECT--
67+
==DONE==

Zend/zend_gc.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -608,7 +608,7 @@ static void gc_adjust_threshold(int count)
608608
/* TODO Very simple heuristic for dynamic GC buffer resizing:
609609
* If there are "too few" collections, increase the collection threshold
610610
* by a fixed step */
611-
if (count < GC_THRESHOLD_TRIGGER) {
611+
if (count < GC_THRESHOLD_TRIGGER || GC_G(num_roots) >= GC_G(gc_threshold)) {
612612
/* increase */
613613
if (GC_G(gc_threshold) < GC_THRESHOLD_MAX) {
614614
new_threshold = GC_G(gc_threshold) + GC_THRESHOLD_STEP;
@@ -1990,7 +1990,11 @@ ZEND_API int zend_gc_collect_cycles(void)
19901990

19911991
finish:
19921992
zend_get_gc_buffer_release();
1993+
1994+
GC_G(gc_active) = 1;
19931995
zend_gc_check_root_tmpvars();
1996+
GC_G(gc_active) = 0;
1997+
19941998
GC_G(collector_time) += zend_hrtime() - start_time;
19951999
return total_count;
19962000
}

0 commit comments

Comments
 (0)