Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » pdo_sqlite fd leak »

pdo_sqlite fd leak

php.internals

Rasmus Lerdorf

9 years ago
I have been chasing a really odd fd leak which I haven't been able to reproduce manually. The code involved is about as simple as you can get: class C { public static function fn($arg) { $pdo = new PDO('sqlite:/var/dbs/file.db'); $query = $pdo->prepare("SELECT * FROM tb WHERE id = ?"); $query->execute([$arg]); $result = $query->fetch(PDO::FETCH_ASSOC); if (!$result) { throw new RuntimeException("not found" ); } return [ "a" => $result['a'], "b" => $result['b'] ]; } } The symptoms are: - It always starts with a max_execution timeout in the *$pdo->prepare()* call - Many hours after this timeout the httpd process runs out of FDs and lsof shows the httpd process with ~1000 fds open on */var/dbs/file.db* The file.db itself is only read from PHP and isn't being updated in the background by anything. mtime is months ago, but this happens sporadically on a single server out of dozens about once every couple of days. These servers are getting hit hard so it takes hundreds of millions of requests to trigger whatever condition is causing this. It feels like it should be something related to timeouts and the *sqlite_handle_closer() *call: https://github.com/php/php-src/blob/PHP-7.0/ext/pdo_sqlite/sqlite_driver.c#L155-L175 but I have been staring at this code and comparing it to the other pdo drivers and I don't see what makes *pdo_sqlite* special here. This is on PHP 7.0.10 but this code hasn't changed significantly in 7.1 and master. Anyone see anything in PDO that could possibly prevent the *sqlite3_close()* call from being executed, especially in edge conditions around an execution timeout? -Rasmus

Nikita Popov

9 years ago
On Fri, Jan 20, 2017 at 9:13 PM, Rasmus Lerdorf <rasmus@lerdorf.com> wrote:
> I have been chasing a really odd fd leak which I haven't been able to > reproduce manually. The code involved is about as simple as you can get: > > class C { > public static function fn($arg) { > $pdo = new PDO('sqlite:/var/dbs/file.db'); > $query = $pdo->prepare("SELECT * FROM tb WHERE id = ?"); > $query->execute([$arg]); > $result = $query->fetch(PDO::FETCH_ASSOC); > > if (!$result) { > throw new RuntimeException("not found" ); > } > > return [ > "a" => $result['a'], > "b" => $result['b'] > ]; > } > } > > The symptoms are: > > - It always starts with a max_execution timeout in the *$pdo->prepare()* > call > - Many hours after this timeout the httpd process runs out of FDs and > lsof shows the httpd process with ~1000 fds open on */var/dbs/file.db* > > The file.db itself is only read from PHP and isn't being updated in the > background by anything. mtime is months ago, but this happens sporadically > on a single server out of dozens about once every couple of days. These > servers are getting hit hard so it takes hundreds of millions of requests > to trigger whatever condition is causing this. > > It feels like it should be something related to timeouts and the > *sqlite_handle_closer() > *call: > > https://github.com/php/php-src/blob/PHP-7.0/ext/pdo_ > sqlite/sqlite_driver.c#L155-L175 > > but I have been staring at this code and comparing it to the other pdo > drivers and I don't see what makes *pdo_sqlite* special here. > This is on PHP 7.0.10 but this code hasn't changed significantly in 7.1 and > master. Anyone see anything in PDO that could possibly prevent the > *sqlite3_close()* call from being executed, especially in edge conditions > around an execution timeout? > > -Rasmus >
From the docs for sqlite3_close():
> If the database connection is associated with unfinalized prepared
statements or unfinished sqlite3_backup objects then sqlite3_close() will leave the database connection open and return SQLITE_BUSY <http://www.sqlite.org/rescode.html#busy>. If sqlite3_close_v2() is called with unfinalized prepared statements and/or unfinished sqlite3_backups, then the database connection becomes an unusable "zombie" which will automatically be deallocated when the last prepared statement is finalized or the last sqlite3_backup is finished. The sqlite3_close_v2() interface is intended for use with host languages that are garbage collected, and where the order in which destructors are called is arbitrary. That sounds like it could be the source of the issue. Nikita

Rasmus Lerdorf

9 years ago
On Fri, Jan 20, 2017 at 12:58 Nikita Popov <nikita.ppv@gmail.com> wrote:
> > From the docs for sqlite3_close(): > > > If the database connection is associated with unfinalized prepared > > statements or unfinished sqlite3_backup objects then sqlite3_close() > > will leave the database connection open and return SQLITE_BUSY > <http://www.sqlite.org/rescode.html#busy>. > > If sqlite3_close_v2() is called with unfinalized prepared statements > > and/or unfinished sqlite3_backups, then the database connection becomes > > an unusable "zombie" which will automatically be deallocated when the > > last prepared statement is finalized or the last sqlite3_backup is > > finished. The sqlite3_close_v2() interface is intended for use with > > host languages that are garbage collected, and where the order in which > > destructors are called is arbitrary. > > > > > That sounds like it could be the source of the issue. >
Ah, that makes more sense that it never hitting that close call because I couldn't find any scenario where we wouldn't get there eventually. So it sounds like we should be calling sqlite3_close_v2() there instead. -Rasmus

Rasmus Lerdorf

9 years ago
On Fri, Jan 20, 2017 at 1:08 PM, Rasmus Lerdorf <rasmus@lerdorf.com> wrote:
> > On Fri, Jan 20, 2017 at 12:58 Nikita Popov <nikita.ppv@gmail.com> wrote: > >> >> That sounds like it could be the source of the issue. >> > Ah, that makes more sense than it never hitting that close call because I > couldn't find any scenario where we wouldn't get there eventually. So it > sounds like we should be calling sqlite3_close_v2() there instead. >
Of course, something must be causing the unfinalized prepared statement in the first place so moving to the v2 close likely wouldn't fix it, just move it from an unclosed db handle to an unclosed "unusable zombie" handle, whatever that means. I also noticed that ext/sqlite3 uses sqlite3_prepare_v2() while pdo_sqlite uses sqlite3_prepare(). The differences in those two don't seem like they would affect whether the prepare is finalized or not though. There still must be some path where on a timeout we don't call pdo_sqlite_stmt_dtor() which does the finalize on the statement. -Rasmus

Christoph Becker

9 years ago
On 20.01.2017 at 22:41, Rasmus Lerdorf wrote:
> On Fri, Jan 20, 2017 at 1:08 PM, Rasmus Lerdorf <rasmus@lerdorf.com> wrote: > >> On Fri, Jan 20, 2017 at 12:58 Nikita Popov <nikita.ppv@gmail.com> wrote: >> >>> That sounds like it could be the source of the issue. >> >> Ah, that makes more sense than it never hitting that close call because I >> couldn't find any scenario where we wouldn't get there eventually. So it >> sounds like we should be calling sqlite3_close_v2() there instead. > > Of course, something must be causing the unfinalized prepared statement in > the first place so moving to the v2 close likely wouldn't fix it, just move > it from an unclosed db handle to an unclosed "unusable zombie" handle, > whatever that means. I also noticed that ext/sqlite3 uses > sqlite3_prepare_v2() while pdo_sqlite uses sqlite3_prepare(). The > differences in those two don't seem like they would affect whether the > prepare is finalized or not though.
Anyhow, the SQLite3 documentation states[1]: | The sqlite3_prepare_v2() and sqlite3_prepare16_v2() interfaces are | recommended for all new programs. The two older interfaces are | retained for backwards compatibility, but their use is discouraged. Isn't that reason enough to switch to sqlite3_prepare_v2() ASAP? Note that this is documented at least for more than nine years[2]! [1] <https://sqlite.org/c3ref/prepare.html> [2] <http://web.archive.org/web/20070701000000*/https://sqlite.org/c3ref/prepare.html>
-- Christoph M. Becker

Rasmus Lerdorf

9 years ago
On Sat, Jan 21, 2017 at 4:47 PM, Christoph M. Becker <cmbecker69@gmx.de> wrote:
> > Anyhow, the SQLite3 documentation states[1]: > > | The sqlite3_prepare_v2() and sqlite3_prepare16_v2() interfaces are > | recommended for all new programs. The two older interfaces are > | retained for backwards compatibility, but their use is discouraged. > > Isn't that reason enough to switch to sqlite3_prepare_v2() ASAP? Note > that this is documented at least for more than nine years[2]! > > [1] <https://sqlite.org/c3ref/prepare.html> > [2] > <http://web.archive.org/web/20070701000000*/https:// > sqlite.org/c3ref/prepare.html>
Yes, but it also says that the behaviour is slightly different on an error condition which could potentially affect peoples' code. Although it seems like a subtle difference and only in the case of an error, so it should be ok to change for 7.2. -Rasmus

Rasmus Lerdorf

9 years ago
On Sat, Jan 21, 2017 at 5:22 PM, Rasmus Lerdorf <rasmus@lerdorf.com> wrote:
> On Sat, Jan 21, 2017 at 4:47 PM, Christoph M. Becker <cmbecker69@gmx.de> > wrote: >> >> Anyhow, the SQLite3 documentation states[1]: >> >> | The sqlite3_prepare_v2() and sqlite3_prepare16_v2() interfaces are >> | recommended for all new programs. The two older interfaces are >> | retained for backwards compatibility, but their use is discouraged. >> >> Isn't that reason enough to switch to sqlite3_prepare_v2() ASAP? Note >> that this is documented at least for more than nine years[2]! >> >> [1] <https://sqlite.org/c3ref/prepare.html> >> [2] >> <http://web.archive.org/web/20070701000000*/https://sqlite. >> org/c3ref/prepare.html> > > > Yes, but it also says that the behaviour is slightly different on an error > condition which could potentially affect peoples' code. Although it seems > like a subtle difference and only in the case of an error, so it should be > ok to change for 7.2. >
Ok, I have switched pdo_sqlite to use sqlite3_prepare_v2() and sqlite3_close_v2() for PHP 7.2