Skip to content

Commit fb56641

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 059bf33 commit fb56641

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
@@ -557,7 +557,7 @@ static void gc_adjust_threshold(int count)
557557
/* TODO Very simple heuristic for dynamic GC buffer resizing:
558558
* If there are "too few" collections, increase the collection threshold
559559
* by a fixed step */
560-
if (count < GC_THRESHOLD_TRIGGER) {
560+
if (count < GC_THRESHOLD_TRIGGER || GC_G(num_roots) >= GC_G(gc_threshold)) {
561561
/* increase */
562562
if (GC_G(gc_threshold) < GC_THRESHOLD_MAX) {
563563
new_threshold = GC_G(gc_threshold) + GC_THRESHOLD_STEP;
@@ -1674,7 +1674,11 @@ ZEND_API int zend_gc_collect_cycles(void)
16741674

16751675
finish:
16761676
zend_get_gc_buffer_release();
1677+
1678+
GC_G(gc_active) = 1;
16771679
zend_gc_check_root_tmpvars();
1680+
GC_G(gc_active) = 0;
1681+
16781682
return total_count;
16791683
}
16801684

0 commit comments

Comments
 (0)