Skip to content

Commit 52ada4d

Browse files
Craig Phillipsmeta-codesync[bot]
authored andcommitted
Fix file descriptor leak when SQLite3 objects are garbage collected
Summary: The SQLite3 native data classes were registered with NO_SWEEP flag, which prevented their sweep methods from being called during request shutdown. This caused file descriptors to leak when PHP code didn't explicitly call $db->close() and the objects survived until request end. Observed this in prod when comparing PDO and SQLite file descriptor usage https://www.internalfb.com/phabricator/paste/view/P2169381411 # This Change 1. Adds sweep() methods to SQLite3, SQLite3Stmt, and SQLite3Result that properly clean up native SQLite resources 2. Updates destructors to call sweep() for consistent cleanup 3. Removes the NO_SWEEP flag so sweep is called during request shutdown The sweep() implementation follows the same pattern used by PDO SQLite, including proper handling of UDF Variants with releaseForSweep(). # Why NO_SWEEP was wrong NO_SWEEP doesn't defer cleanup - it PREVENTS cleanup entirely for objects that survive until request end. The memory is bulk-freed without calling any cleanup functions, so native resources (file descriptors) leak. Reviewed By: paulbiss Differential Revision: D92496862 fbshipit-source-id: 5ed26749e770cc99c845b9b5741d9b4ab12b75a6
1 parent 8ef8458 commit 52ada4d

2 files changed

Lines changed: 10 additions & 6 deletions

File tree

hphp/runtime/ext/sqlite3/ext_sqlite3.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,10 @@ static void php_sqlite3_callback_final(sqlite3_context *context) {
167167
SQLite3::SQLite3() : m_raw_db(nullptr) {
168168
}
169169

170-
SQLite3::~SQLite3() {
170+
void SQLite3::sweep() {
171171
if (m_raw_db) {
172172
sqlite3_close_v2(m_raw_db);
173+
m_raw_db = nullptr;
173174
}
174175
}
175176

@@ -485,9 +486,10 @@ bool HHVM_METHOD(SQLite3, openblob, const String& /*table*/,
485486
SQLite3Stmt::SQLite3Stmt() : m_raw_stmt(nullptr) {
486487
}
487488

488-
SQLite3Stmt::~SQLite3Stmt() {
489+
void SQLite3Stmt::sweep() {
489490
if (m_raw_stmt) {
490491
sqlite3_finalize(m_raw_stmt);
492+
m_raw_stmt = nullptr;
491493
}
492494
}
493495

@@ -774,7 +776,7 @@ static struct SQLite3Extension final : Extension {
774776
HHVM_ME(SQLite3, openblob);
775777
HHVM_STATIC_ME(SQLite3, version);
776778
HHVM_STATIC_ME(SQLite3, escapestring);
777-
Native::registerNativeDataInfo<SQLite3>(Native::NDIFlags::NO_SWEEP);
779+
Native::registerNativeDataInfo<SQLite3>();
778780

779781
HHVM_ME(SQLite3Stmt, __construct);
780782
HHVM_ME(SQLite3Stmt, paramcount);
@@ -783,7 +785,7 @@ static struct SQLite3Extension final : Extension {
783785
HHVM_ME(SQLite3Stmt, clear);
784786
HHVM_ME(SQLite3Stmt, bindvalue);
785787
HHVM_ME(SQLite3Stmt, execute);
786-
Native::registerNativeDataInfo<SQLite3Stmt>(Native::NDIFlags::NO_SWEEP);
788+
Native::registerNativeDataInfo<SQLite3Stmt>();
787789

788790
HHVM_ME(SQLite3Result, numcolumns);
789791
HHVM_ME(SQLite3Result, columnname);

hphp/runtime/ext/sqlite3/ext_sqlite3.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ namespace HPHP {
2626

2727
struct SQLite3 : SystemLib::ClassLoader<"SQLite3"> {
2828
SQLite3();
29-
~SQLite3();
29+
~SQLite3() { sweep(); }
30+
void sweep();
3031
void validate() const;
3132

3233
struct UserDefinedFunc {
@@ -51,7 +52,8 @@ void HHVM_METHOD(SQLite3, open,
5152

5253
struct SQLite3Stmt : SystemLib::ClassLoader<"SQLite3Stmt"> {
5354
SQLite3Stmt();
54-
~SQLite3Stmt();
55+
~SQLite3Stmt() { sweep(); }
56+
void sweep();
5557
void validate() const;
5658

5759
struct BoundParam {

0 commit comments

Comments
 (0)