Hi Guys,
PHP_FUNCTION(proc_close) doesn't have a call to
CloseHandle(proc->child), to close the process handle.
This is causing a handle leak on Windows, and eventually brings the
whole OS to it's knees.
Here's the fixed code snippet (there's a DIFF at the end of the file):
------------- cut here -------------
/* {{{ proto int proc_close(resource process)
close a process opened by proc_open */
PHP_FUNCTION(proc_close)
{
zval *zproc;
struct php_process_handle *proc;
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "r", &zproc) ==
FAILURE) {
RETURN_FALSE;
}
ZEND_FETCH_RESOURCE(proc, struct php_process_handle *, &zproc, -1,
"process", le_proc_open);
CloseHandle(proc->child);// ilya.1.0 20041110
zend_list_delete(Z_LVAL_P(zproc));
RETURN_LONG(FG(pclose_ret));
}
/* }}} */
------------- cut here -------------
Index: php-src/ext/standard/proc_open.c
RCS file: /repository/php-src/ext/standard/proc_open.c,v
retrieving revision 1.29
diff -r1.29 proc_open.c
90c90
<
94c94
<
96c96
<
110c110
<
116c116
<
118c118
<
137c137
<
140c140
<
144c144
<
155c155
<
173c173
< }
}
176c176
<
216c216
<
218c218
<
222c222
<
224c224
<
228c228
<
236c236
<
243c243
<
264c264
<
266c266
<
282c282
<
324c324
<
330c330
<
332a333,334
/* should use this? if a handle leak occurs - should */ // CloseHandle(proc->child); // ilya.1.0 20041110
336c338
<
348c350
<
354c356,358
<
CloseHandle(proc->child); // ilya.1.0 20041110
374c378
<
385c389
<
387c391
<
392c396
<
394c398
<
397c401
<
433c437
<
518c522
< }
}
520c524
<
538c542
<
621a626
630c635
<
710c715
<
714c719
<
730c735
<
732c737
<
739c744
<
745c750
<
778c783
<
800c805
<
804c809
<
924a930
930c936
<
Hi Guys,
PHP_FUNCTION(proc_close) doesn't have a call to
CloseHandle(proc->child), to close the process handle.
This is causing a handle leak on Windows, and eventually brings the
whole OS to it's knees.Here's the fixed code snippet (there's a DIFF at the end of the file):
Can you please post a unified diff without any whitespace changes (and
put it online and provide a link, as gmail mangles files it seems).
Derick
--
Derick Rethans
http://derickrethans.nl | http://ez.no | http://xdebug.org
On Wed, 10 Nov 2004 17:49:45 +0200
"ilya77@gmail.com" ilya77@gmail.com wrote:
Hi Guys,
PHP_FUNCTION(proc_close) doesn't have a call to
CloseHandle(proc->child), to close the process handle.
This is causing a handle leak on Windows, and eventually brings the
whole OS to it's knees.Here's the fixed code snippet (there's a DIFF at the end of the file):
CloseHandle(proc->child);// ilya.1.0 20041110
First of all, this will work only under M$ systems and there are
plenty of others, that will be broken by your patch.
Second, handles are destroyed by proc_open_rsrc_dtor(), so I'd
propose the patch below instead:
Index: proc_open.c
RCS file: /repository/php-src/ext/standard/proc_open.c,v
retrieving revision 1.29
diff -u -r1.29 proc_open.c
--- proc_open.c 29 Sep 2004 06:04:36 -0000 1.29
+++ proc_open.c 10 Nov 2004 16:03:32 -0000
@@ -219,6 +219,7 @@
WaitForSingleObject(proc->child, INFINITE);
GetExitCodeProcess(proc->child, &wstatus);
FG(pclose_ret) = wstatus;
-
CloseHandle(proc->child);
#elif HAVE_SYS_WAIT_H
--
Wbr,
Antony Dovgal aka tony2001
tony2001@phpclub.net || antony@dovgal.com
Hi Antony,
Makes sense to me, however, what about TerminateProcess() in
PHP_FUNCTION(proc_terminate)?
As far as I recall (don't take my word for it), TerminateProcess()
closes the handle for you, am I missing something? Can it break things
on future releases of the OS?
Thanks!
On Wed, 10 Nov 2004 17:49:45 +0200
"ilya77@gmail.com" ilya77@gmail.com wrote:
Hi Guys,
PHP_FUNCTION(proc_close) doesn't have a call to
CloseHandle(proc->child), to close the process handle.
This is causing a handle leak on Windows, and eventually brings the
whole OS to it's knees.Here's the fixed code snippet (there's a DIFF at the end of the file):
CloseHandle(proc->child);// ilya.1.0 20041110
First of all, this will work only under M$ systems and there are
plenty of others, that will be broken by your patch.
Second, handles are destroyed by proc_open_rsrc_dtor(), so I'd
propose the patch below instead:Index: proc_open.c
RCS file: /repository/php-src/ext/standard/proc_open.c,v
retrieving revision 1.29
diff -u -r1.29 proc_open.c
--- proc_open.c 29 Sep 2004 06:04:36 -0000 1.29
+++ proc_open.c 10 Nov 2004 16:03:32 -0000
@@ -219,6 +219,7 @@
WaitForSingleObject(proc->child, INFINITE);
GetExitCodeProcess(proc->child, &wstatus);
FG(pclose_ret) = wstatus;
CloseHandle(proc->child);#elif HAVE_SYS_WAIT_H
--
Wbr,
Antony Dovgal aka tony2001
tony2001@phpclub.net || antony@dovgal.com
proc_terminate should set the child handle to INVALID_HANDLE_VALUE after
it closes the handle, and the dtor should check that the child handle is
valid before it closes it.
--Wez.
ilya77@gmail.com wrote:
Hi Antony,
Makes sense to me, however, what about TerminateProcess() in
PHP_FUNCTION(proc_terminate)?
As far as I recall (don't take my word for it), TerminateProcess()
closes the handle for you, am I missing something? Can it break things
on future releases of the OS?PHP_FUNCTION(proc_close) doesn't have a call to
CloseHandle(proc->child), to close the process handle.
This is causing a handle leak on Windows
No no, I've checked the documentation.
It seems that CloseHandle() is still required after terminating a process.
Although MS documentation for it's own products is not always 100% accurate.
I suggest checking this impirically...
proc_terminate should set the child handle to INVALID_HANDLE_VALUE after
it closes the handle, and the dtor should check that the child handle is
valid before it closes it.--Wez.
ilya77@gmail.com wrote:
Hi Antony,
Makes sense to me, however, what about TerminateProcess() in
PHP_FUNCTION(proc_terminate)?
As far as I recall (don't take my word for it), TerminateProcess()
closes the handle for you, am I missing something? Can it break things
on future releases of the OS?PHP_FUNCTION(proc_close) doesn't have a call to
CloseHandle(proc->child), to close the process handle.
This is causing a handle leak on Windows
Fixed in CVS.