Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » [PATCH] Bug #30743 - proc_close() causes a handle leak »

[PATCH] Bug #30743 - proc_close() causes a handle leak

php.internals

ilya77@gmail.com

21 years ago
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 < ---

Derick Rethans

21 years ago
On Wed, 10 Nov 2004, 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):
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

Antony Dovgal

21 years ago
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

ilya77@gmail.com

21 years ago
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 19:08:18 +0300, Antony Dovgal <tony2001@phpclub.net> wrote:

Wez Furlong

21 years ago
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:

ilya77@gmail.com

21 years ago
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... On Wed, 10 Nov 2004 13:54:08 -0500, Wez Furlong <wez@thebrainroom.com> wrote:

Wez Furlong

21 years ago
Fixed in CVS.