Skip to content

Commit eee0ec8

Browse files
committed
Do not inherit open FDs to SQLite worker by overwriting and closing
This somewhat obscure PR ensures that we do not inherit open file descriptors (FDs) to the SQLite child worker process. This can cause all sorts of errors in long running applications and really is not desired here. This is implemented by explicitly overwriting all superfluous FDs with dummy file handles and then closing all of these in the implicit `sh` child process before launching the actual php binary. PHP does not support `FD_CLOEXEC`, `O_CLOEXEC` or `SOCK_CLOEXEC` and this appears to be the best work around I could find (yes, I should probably write a lengthy, somewhat technical blog post about this). Additionally, this PR includes a test to verify this works on all supported platforms and this could perhaps be used as a starting point for other libraries (YMMV). This builds on top of clue/reactphp-ssh-proxy#2 and clue/reactphp-ssh-proxy#10
1 parent 22b1de8 commit eee0ec8

File tree

2 files changed

+94
-1
lines changed

2 files changed

+94
-1
lines changed

src/Database.php

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,49 @@ class Database extends EventEmitter
8383
*/
8484
public static function open(LoopInterface $loop, $filename, $flags = null)
8585
{
86-
$process = new Process('exec ' . \escapeshellarg(\PHP_BINARY) . ' ' . \escapeshellarg(__DIR__ . '/../res/sqlite-worker.php'));
86+
$command = 'exec ' . \escapeshellarg(\PHP_BINARY) . ' ' . \escapeshellarg(__DIR__ . '/../res/sqlite-worker.php');
87+
88+
// Try to get list of all open FDs (Linux/Mac and others)
89+
$fds = @\scandir('/dev/fd');
90+
91+
// Otherwise try temporarily duplicating file descriptors in the range 0-1024 (FD_SETSIZE).
92+
// This is known to work on more exotic platforms and also inside chroot
93+
// environments without /dev/fd. Causes many syscalls, but still rather fast.
94+
// @codeCoverageIgnoreStart
95+
if ($fds === false) {
96+
$fds = array();
97+
for ($i = 0; $i <= 1024; ++$i) {
98+
$copy = @\fopen('php://fd/' . $i, 'r');
99+
if ($copy !== false) {
100+
$fds[] = $i;
101+
\fclose($copy);
102+
}
103+
}
104+
}
105+
// @codeCoverageIgnoreEnd
106+
107+
// launch process with default STDIO pipes
108+
$pipes = array(
109+
array('pipe', 'r'),
110+
array('pipe', 'w'),
111+
array('pipe', 'w')
112+
);
113+
114+
// do not inherit open FDs by explicitly overwriting existing FDs with dummy files
115+
// additionally, close all dummy files in the child process again
116+
foreach ($fds as $fd) {
117+
if ($fd > 2) {
118+
$pipes[$fd] = array('file', '/dev/null', 'r');
119+
$command .= ' ' . $fd . '>&-';
120+
}
121+
}
122+
123+
// default `sh` only accepts single-digit FDs, so run in bash if needed
124+
if ($fds && \max($fds) > 9) {
125+
$command = 'exec bash -c ' . \escapeshellarg($command);
126+
}
127+
128+
$process = new Process($command, null, null, $pipes);
87129
$process->start($loop);
88130

89131
$db = new Database($process);

tests/FunctionalDatabaseTest.php

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,33 @@ public function testOpenMemoryDatabaseResolvesWithDatabaseAndRunsUntilQuit()
4141
$loop->run();
4242
}
4343

44+
public function testOpenMemoryDatabaseShouldNotInheritActiveFileDescriptors()
45+
{
46+
$server = stream_socket_server('tcp://127.0.0.1:0');
47+
$address = stream_socket_get_name($server, false);
48+
49+
if (@stream_socket_server('tcp://' . $address) !== false) {
50+
$this->markTestSkipped('Platform does not prevent binding to same address (Windows?)');
51+
}
52+
53+
$loop = Factory::create();
54+
55+
$promise = Database::open($loop, ':memory:');
56+
57+
// close server and ensure we can start a new server on the previous address
58+
// the pending SQLite process should not inherit the existing server socket
59+
fclose($server);
60+
$server = stream_socket_server('tcp://' . $address);
61+
$this->assertTrue(is_resource($server));
62+
fclose($server);
63+
64+
$promise->then(function (Database $db) {
65+
$db->close();
66+
});
67+
68+
$loop->run();
69+
}
70+
4471
public function testOpenInvalidPathRejects()
4572
{
4673
$loop = Factory::create();
@@ -83,6 +110,30 @@ public function testQuitResolvesAndRunsUntilQuit()
83110
$loop->run();
84111
}
85112

113+
114+
public function testQuitResolvesAndRunsUntilQuitWhenParentHasManyFileDescriptors()
115+
{
116+
$servers = array();
117+
for ($i = 0; $i < 100; ++$i) {
118+
$servers[] = stream_socket_server('tcp://127.0.0.1:0');
119+
}
120+
121+
$loop = Factory::create();
122+
123+
$promise = Database::open($loop, ':memory:');
124+
125+
$once = $this->expectCallableOnce();
126+
$promise->then(function (Database $db) use ($once){
127+
$db->quit()->then($once);
128+
});
129+
130+
$loop->run();
131+
132+
foreach ($servers as $server) {
133+
fclose($server);
134+
}
135+
}
136+
86137
public function testQuitTwiceWillRejectSecondCall()
87138
{
88139
$loop = Factory::create();

0 commit comments

Comments
 (0)