Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQLite3 callback functions cause a memory leak with a callable array #11878

Closed
youkidearitai opened this issue Aug 5, 2023 · 3 comments
Closed

Comments

@youkidearitai
Copy link
Contributor

Description

I'm trying phpdoc on http://doc.php.net/tutorial/local-setup.php, then displays memory leak. (My PHP environment is include --enable-debug)

$ ~/php_master/bin/php -i | less
Configure Command =>  './configure'  '--enable-mbstring' '--enable-debug' '--prefix=/Users/tekimen/php_master' '--with-iconv=/opt/homebrew/opt/libiconv' '--enable-bcmath' '--with-zlib'

The following code:

~/php_master/bin/php -d 'ini_set("memory_limit", -1);'  phd/render.php --docbook doc-base/.manual.xml --package PHP --format xhtml

Resulted in this output:

PHP:  syntax error, unexpected '(' in Unknown on line 7
[02:40:04 - Rendering Style       ] Running full build
[02:40:04 - Indexing              ] Skipping indexing
[02:40:04 - Rendering Style       ] Running full build
[02:40:04 - Rendering Format      ] Starting PHP-Chunked-XHTML rendering
[02:40:05 - E_USER_WARNING        ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:635
	Impossible to copy the http://www.php.net/styles/theme-base.css file.
[02:40:05 - E_USER_WARNING        ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:635
	Impossible to copy the http://www.php.net/styles/theme-medium.css file.
[02:40:06 - E_DEPRECATED          ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:915
	Increment on non-alphanumeric string is deprecated
[02:40:06 - E_DEPRECATED          ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:915
	Increment on non-alphanumeric string is deprecated
[02:40:06 - E_DEPRECATED          ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:915
	Increment on non-alphanumeric string is deprecated
[02:40:06 - E_DEPRECATED          ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:915
	Increment on non-alphanumeric string is deprecated
[02:40:06 - E_DEPRECATED          ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:915
	Increment on non-alphanumeric string is deprecated
[02:40:06 - E_DEPRECATED          ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:915
	Increment on non-alphanumeric string is deprecated
[02:40:13 - E_DEPRECATED          ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:915
	Increment on non-alphanumeric string is deprecated
[02:40:18 - E_DEPRECATED          ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:915
	Increment on non-alphanumeric string is deprecated
[02:40:21 - Rendering Format      ] Writing search indexes..
[02:40:21 - Rendering Format      ] Index written
[02:40:21 - Rendering Format      ] Finished rendering
[Sat Aug  5 11:40:21 2023]  Script:  '/Users/tekimen/src/phpdoc/phd/render.php'
/Users/tekimen/src/php-src/Zend/zend_objects_API.h(92) :  Freeing 0x00000001020f32a0 (176 bytes), script=/Users/tekimen/src/phpdoc/phd/render.php
[Sat Aug  5 11:40:21 2023]  Script:  '/Users/tekimen/src/phpdoc/phd/render.php'
/Users/tekimen/src/php-src/Zend/zend_objects.c(187) :  Freeing 0x0000000105d08f00 (104 bytes), script=/Users/tekimen/src/phpdoc/phd/render.php
Last leak repeated 3 times
=== Total 5 memory leaks detected ===

But I expected this output instead:

I expected displays nothing about memory leak.

PHP:  syntax error, unexpected '(' in Unknown on line 7
[02:40:04 - Rendering Style       ] Running full build
[02:40:04 - Indexing              ] Skipping indexing
[02:40:04 - Rendering Style       ] Running full build
[02:40:04 - Rendering Format      ] Starting PHP-Chunked-XHTML rendering
[02:40:05 - E_USER_WARNING        ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:635
	Impossible to copy the http://www.php.net/styles/theme-base.css file.
[02:40:05 - E_USER_WARNING        ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:635
	Impossible to copy the http://www.php.net/styles/theme-medium.css file.
[02:40:06 - E_DEPRECATED          ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:915
	Increment on non-alphanumeric string is deprecated
[02:40:06 - E_DEPRECATED          ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:915
	Increment on non-alphanumeric string is deprecated
[02:40:06 - E_DEPRECATED          ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:915
	Increment on non-alphanumeric string is deprecated
[02:40:06 - E_DEPRECATED          ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:915
	Increment on non-alphanumeric string is deprecated
[02:40:06 - E_DEPRECATED          ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:915
	Increment on non-alphanumeric string is deprecated
[02:40:06 - E_DEPRECATED          ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:915
	Increment on non-alphanumeric string is deprecated
[02:40:13 - E_DEPRECATED          ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:915
	Increment on non-alphanumeric string is deprecated
[02:40:18 - E_DEPRECATED          ] /Users/tekimen/src/phpdoc/phd/phpdotnet/phd/Package/Generic/XHTML.php:915
	Increment on non-alphanumeric string is deprecated
[02:40:21 - Rendering Format      ] Writing search indexes..
[02:40:21 - Rendering Format      ] Index written
[02:40:21 - Rendering Format      ] Finished rendering

PHP Version

PHP 8.3.0 (master)

Operating System

macOS 13.4.1

@nielsdos
Copy link
Member

nielsdos commented Aug 5, 2023

Hmm. Applying GH-11850 does not solve the issue.
EDIT: this is unrelated to the increment/decrement changes, this also reproduces on 8.1 & 8.2

@nielsdos
Copy link
Member

nielsdos commented Aug 5, 2023

I managed to build a reproducer that reproduces the underlying issue on 8.1 and higher.

<?php
class Foo {
    public $sqlite;
    public function __construct() {
        $this->sqlite = new SQLite3(":memory:");
        $this->sqlite->createAggregate("indexes", array($this, "SQLiteIndex"), array($this, "SQLiteFinal"), 9);
    }
    public function SQLiteIndex() {}
    public function SQLiteFinal() {}
}

$x = new Foo;

@nielsdos
Copy link
Member

nielsdos commented Aug 5, 2023

I think the problem is that we ZVAL_COPY the function name from the fci, but the fci is never destroyed. This means the refcount increase of the fci will never be decreased.
A simple fix would be to transfer the ownership of the function names, i.e. use ZVAL_COPY_VALUE instead of ZVAL_COPY.
I'll try that.
EDIT: no, that doesn't actually fix it properly
EDIT 2: I think I got it, and I think the root cause is an engine issue, so relabelling that...

@nielsdos nielsdos changed the title Build phpdoc memory leak detected on master branch SQLite3 callback functions cause a memory leak with a callable array Aug 5, 2023
nielsdos added a commit to nielsdos/php-src that referenced this issue Aug 5, 2023
…a callable array

In this test file, the free_obj handler is called with a refcount of 2,
caused by the fact it participates in a cycle and we do a GC_ADDREF() to
increase its refcount. We never decrease its refcount so it never
becomes 0 causing a memory leak.
This happens in shutdown. Whether it is actually useful to check the
refcount afterwards is up for debate.
nielsdos added a commit to nielsdos/php-src that referenced this issue Sep 8, 2023
…a callable array

In this test file, the free_obj handler is called with a refcount of 2,
caused by the fact we do a GC_ADDREF() to increase its refcount while
its refcount is still 1 because the Foo object hasn't been destroyed yet
(due to the cycle caused by the sqlite function callback).
Solve this by introducing a get_gc handler.

Closes phpGH-11881.
nielsdos added a commit to nielsdos/php-src that referenced this issue Sep 8, 2023
…a callable array

In this test file, the free_obj handler is called with a refcount of 2,
caused by the fact we do a GC_ADDREF() to increase its refcount while
its refcount is still 1 because the Foo object hasn't been destroyed yet
(due to the cycle caused by the sqlite function callback).
Solve this by introducing a get_gc handler.

Closes phpGH-11881.
nielsdos added a commit that referenced this issue Sep 9, 2023
* PHP-8.1:
  Fix GH-11878: SQLite3 callback functions cause a memory leak with a callable array
nielsdos added a commit that referenced this issue Sep 9, 2023
* PHP-8.2:
  Fix GH-11878: SQLite3 callback functions cause a memory leak with a callable array
nielsdos added a commit that referenced this issue Sep 9, 2023
* PHP-8.3:
  Fix GH-11878: SQLite3 callback functions cause a memory leak with a callable array
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants