[Python-checkins] CVS: python/dist/src/Modules posixmodule.c,2.163,2.164
Mark Hammond
python-dev@python.org
Sun, 13 Aug 2000 21:47:36 -0700
Update of /cvsroot/python/python/dist/src/Modules
In directory slayer.i.sourceforge.net:/tmp/cvs-serv23896
Modified Files:
posixmodule.c
Log Message:
Patch #101032, from David Bolen:
This is an enhancement to a prior patch (100941) ...
[T]his patch removes the risk of deadlock waiting for the child previously present in certain cases. It adds tracking of all file handles returned from an os.popen* call and only waits for the child process, returning the exit code, on the closure of the final file handle to that child.
Index: posixmodule.c
===================================================================
RCS file: /cvsroot/python/python/dist/src/Modules/posixmodule.c,v
retrieving revision 2.163
retrieving revision 2.164
diff -C2 -r2.163 -r2.164
*** posixmodule.c 2000/07/31 15:28:04 2.163
--- posixmodule.c 2000/08/14 04:47:33 2.164
***************
*** 2099,2103 ****
* Written by Bill Tutt <billtut@microsoft.com>. Minor tweaks
* and 2.0 integration by Fredrik Lundh <fredrik@pythonware.com>
! * Return code handling by David Bolen.
*/
--- 2099,2103 ----
* Written by Bill Tutt <billtut@microsoft.com>. Minor tweaks
* and 2.0 integration by Fredrik Lundh <fredrik@pythonware.com>
! * Return code handling by David Bolen <db3l@fitlinxx.com>.
*/
***************
*** 2117,2122 ****
/*
* Internal dictionary mapping popen* file pointers to process handles,
! * in order to maintain a link to the process handle until the file is
! * closed, at which point the process exit code is returned to the caller.
*/
static PyObject *_PyPopenProcs = NULL;
--- 2117,2122 ----
/*
* Internal dictionary mapping popen* file pointers to process handles,
! * for use when retrieving the process exit code. See _PyPclose() below
! * for more information on this dictionary's use.
*/
static PyObject *_PyPopenProcs = NULL;
***************
*** 2277,2284 ****
static int
! _PyPopenCreateProcess(char *cmdstring, FILE *file,
HANDLE hStdin,
HANDLE hStdout,
! HANDLE hStderr)
{
PROCESS_INFORMATION piProcInfo;
--- 2277,2285 ----
static int
! _PyPopenCreateProcess(char *cmdstring,
HANDLE hStdin,
HANDLE hStdout,
! HANDLE hStderr,
! HANDLE *hProcess)
{
PROCESS_INFORMATION piProcInfo;
***************
*** 2355,2378 ****
CloseHandle(piProcInfo.hThread);
! /*
! * Try to insert our process handle into the internal
! * dictionary so we can find it later when trying
! * to close this file.
! */
! if (!_PyPopenProcs)
! _PyPopenProcs = PyDict_New();
! if (_PyPopenProcs) {
! PyObject *hProcessObj, *fileObj;
!
! hProcessObj = PyLong_FromVoidPtr(piProcInfo.hProcess);
! fileObj = PyLong_FromVoidPtr(file);
!
! if (!hProcessObj || !fileObj ||
! PyDict_SetItem(_PyPopenProcs,
! fileObj, hProcessObj) < 0) {
! /* Insert failure - close handle to prevent leak */
! CloseHandle(piProcInfo.hProcess);
! }
! }
return TRUE;
}
--- 2356,2361 ----
CloseHandle(piProcInfo.hThread);
! /* Return process handle */
! *hProcess = piProcInfo.hProcess;
return TRUE;
}
***************
*** 2387,2391 ****
HANDLE hChildStdinRd, hChildStdinWr, hChildStdoutRd, hChildStdoutWr,
hChildStderrRd, hChildStderrWr, hChildStdinWrDup, hChildStdoutRdDup,
! hChildStderrRdDup; /* hChildStdoutWrDup; */
SECURITY_ATTRIBUTES saAttr;
--- 2370,2374 ----
HANDLE hChildStdinRd, hChildStdinWr, hChildStdoutRd, hChildStdoutWr,
hChildStderrRd, hChildStderrWr, hChildStdinWrDup, hChildStdoutRdDup,
! hChildStderrRdDup, hProcess; /* hChildStdoutWrDup; */
SECURITY_ATTRIBUTES saAttr;
***************
*** 2393,2396 ****
--- 2376,2380 ----
int fd1, fd2, fd3;
FILE *f1, *f2, *f3;
+ long file_count;
PyObject *f;
***************
*** 2491,2494 ****
--- 2475,2479 ----
break;
}
+ file_count = 1;
break;
***************
*** 2513,2517 ****
p1 = PyFile_FromFile(f1, cmdstring, m2, _PyPclose);
PyFile_SetBufSize(p1, 0);
! p2 = PyFile_FromFile(f2, cmdstring, m1, fclose);
PyFile_SetBufSize(p2, 0);
--- 2498,2502 ----
p1 = PyFile_FromFile(f1, cmdstring, m2, _PyPclose);
PyFile_SetBufSize(p1, 0);
! p2 = PyFile_FromFile(f2, cmdstring, m1, _PyPclose);
PyFile_SetBufSize(p2, 0);
***************
*** 2520,2523 ****
--- 2505,2509 ----
f = Py_BuildValue("OO",p1,p2);
+ file_count = 2;
break;
}
***************
*** 2543,2552 ****
f3 = _fdopen(fd3, m1);
p1 = PyFile_FromFile(f1, cmdstring, m2, _PyPclose);
! p2 = PyFile_FromFile(f2, cmdstring, m1, fclose);
! p3 = PyFile_FromFile(f3, cmdstring, m1, fclose);
PyFile_SetBufSize(p1, 0);
PyFile_SetBufSize(p2, 0);
PyFile_SetBufSize(p3, 0);
f = Py_BuildValue("OOO",p1,p2,p3);
break;
}
--- 2529,2539 ----
f3 = _fdopen(fd3, m1);
p1 = PyFile_FromFile(f1, cmdstring, m2, _PyPclose);
! p2 = PyFile_FromFile(f2, cmdstring, m1, _PyPclose);
! p3 = PyFile_FromFile(f3, cmdstring, m1, _PyPclose);
PyFile_SetBufSize(p1, 0);
PyFile_SetBufSize(p2, 0);
PyFile_SetBufSize(p3, 0);
f = Py_BuildValue("OOO",p1,p2,p3);
+ file_count = 3;
break;
}
***************
*** 2555,2573 ****
if (n == POPEN_4) {
if (!_PyPopenCreateProcess(cmdstring,
- f1,
hChildStdinRd,
hChildStdoutWr,
! hChildStdoutWr))
return win32_error("CreateProcess", NULL);
}
else {
if (!_PyPopenCreateProcess(cmdstring,
- f1,
hChildStdinRd,
hChildStdoutWr,
! hChildStderrWr))
return win32_error("CreateProcess", NULL);
}
/* Child is launched. Close the parents copy of those pipe
* handles that only the child should have open. You need to
--- 2542,2645 ----
if (n == POPEN_4) {
if (!_PyPopenCreateProcess(cmdstring,
hChildStdinRd,
hChildStdoutWr,
! hChildStdoutWr,
! &hProcess))
return win32_error("CreateProcess", NULL);
}
else {
if (!_PyPopenCreateProcess(cmdstring,
hChildStdinRd,
hChildStdoutWr,
! hChildStderrWr,
! &hProcess))
return win32_error("CreateProcess", NULL);
}
+ /*
+ * Insert the files we've created into the process dictionary
+ * all referencing the list with the process handle and the
+ * initial number of files (see description below in _PyPclose).
+ * Since if _PyPclose later tried to wait on a process when all
+ * handles weren't closed, it could create a deadlock with the
+ * child, we spend some energy here to try to ensure that we
+ * either insert all file handles into the dictionary or none
+ * at all. It's a little clumsy with the various popen modes
+ * and variable number of files involved.
+ */
+ if (!_PyPopenProcs) {
+ _PyPopenProcs = PyDict_New();
+ }
+
+ if (_PyPopenProcs) {
+ PyObject *procObj, *hProcessObj, *intObj, *fileObj[3];
+ int ins_rc[3];
+
+ fileObj[0] = fileObj[1] = fileObj[2] = NULL;
+ ins_rc[0] = ins_rc[1] = ins_rc[2] = 0;
+
+ procObj = PyList_New(2);
+ hProcessObj = PyLong_FromVoidPtr(hProcess);
+ intObj = PyInt_FromLong(file_count);
+
+ if (procObj && hProcessObj && intObj) {
+ PyList_SetItem(procObj,0,hProcessObj);
+ PyList_SetItem(procObj,1,intObj);
+
+ fileObj[0] = PyLong_FromVoidPtr(f1);
+ if (fileObj[0]) {
+ ins_rc[0] = PyDict_SetItem(_PyPopenProcs,
+ fileObj[0],
+ procObj);
+ }
+ if (file_count >= 2) {
+ fileObj[1] = PyLong_FromVoidPtr(f2);
+ if (fileObj[1]) {
+ ins_rc[1] = PyDict_SetItem(_PyPopenProcs,
+ fileObj[1],
+ procObj);
+ }
+ }
+ if (file_count >= 3) {
+ fileObj[2] = PyLong_FromVoidPtr(f3);
+ if (fileObj[2]) {
+ ins_rc[2] = PyDict_SetItem(_PyPopenProcs,
+ fileObj[2],
+ procObj);
+ }
+ }
+
+ if (ins_rc[0] < 0 || !fileObj[0] ||
+ ins_rc[1] < 0 || (file_count > 1 && !fileObj[1]) ||
+ ins_rc[2] < 0 || (file_count > 2 && !fileObj[2])) {
+ /* Something failed - remove any dictionary
+ * entries that did make it.
+ */
+ if (!ins_rc[0] && fileObj[0]) {
+ PyDict_DelItem(_PyPopenProcs,
+ fileObj[0]);
+ }
+ if (!ins_rc[1] && fileObj[1]) {
+ PyDict_DelItem(_PyPopenProcs,
+ fileObj[1]);
+ }
+ if (!ins_rc[2] && fileObj[2]) {
+ PyDict_DelItem(_PyPopenProcs,
+ fileObj[2]);
+ }
+ }
+ }
+
+ /*
+ * Clean up our localized references for the dictionary keys
+ * and value since PyDict_SetItem will Py_INCREF any copies
+ * that got placed in the dictionary.
+ */
+ Py_XDECREF(procObj);
+ Py_XDECREF(fileObj[0]);
+ Py_XDECREF(fileObj[1]);
+ Py_XDECREF(fileObj[2]);
+ }
+
/* Child is launched. Close the parents copy of those pipe
* handles that only the child should have open. You need to
***************
*** 2591,2594 ****
--- 2663,2684 ----
* Wrapper for fclose() to use for popen* files, so we can retrieve the
* exit code for the child process and return as a result of the close.
+ *
+ * This function uses the _PyPopenProcs dictionary in order to map the
+ * input file pointer to information about the process that was
+ * originally created by the popen* call that created the file pointer.
+ * The dictionary uses the file pointer as a key (with one entry
+ * inserted for each file returned by the original popen* call) and a
+ * single list object as the value for all files from a single call.
+ * The list object contains the Win32 process handle at [0], and a file
+ * count at [1], which is initialized to the total number of file
+ * handles using that list.
+ *
+ * This function closes whichever handle it is passed, and decrements
+ * the file count in the dictionary for the process handle pointed to
+ * by this file. On the last close (when the file count reaches zero),
+ * this function will wait for the child process and then return its
+ * exit code as the result of the close() operation. This permits the
+ * files to be closed in any order - it is always the close() of the
+ * final handle that will return the exit code.
*/
static int _PyPclose(FILE *file)
***************
*** 2597,2613 ****
DWORD exit_code;
HANDLE hProcess;
! PyObject *hProcessObj, *fileObj;
/* Close the file handle first, to ensure it can't block the
! * child from exiting when we wait for it below.
*/
result = fclose(file);
if (_PyPopenProcs) {
! fileObj = PyLong_FromVoidPtr(file);
! if (fileObj) {
! hProcessObj = PyDict_GetItem(_PyPopenProcs, fileObj);
! if (hProcessObj) {
! hProcess = PyLong_AsVoidPtr(hProcessObj);
if (result != EOF &&
WaitForSingleObject(hProcess, INFINITE) != WAIT_FAILED &&
--- 2687,2715 ----
DWORD exit_code;
HANDLE hProcess;
! PyObject *procObj, *hProcessObj, *intObj, *fileObj;
! long file_count;
/* Close the file handle first, to ensure it can't block the
! * child from exiting if it's the last handle.
*/
result = fclose(file);
if (_PyPopenProcs) {
! if ((fileObj = PyLong_FromVoidPtr(file)) != NULL &&
! (procObj = PyDict_GetItem(_PyPopenProcs,
! fileObj)) != NULL &&
! (hProcessObj = PyList_GetItem(procObj,0)) != NULL &&
! (intObj = PyList_GetItem(procObj,1)) != NULL) {
!
! hProcess = PyLong_AsVoidPtr(hProcessObj);
! file_count = PyInt_AsLong(intObj);
!
! if (file_count > 1) {
! /* Still other files referencing process */
! file_count--;
! PyList_SetItem(procObj,1,
! PyInt_FromLong(file_count));
! } else {
! /* Last file for this process */
if (result != EOF &&
WaitForSingleObject(hProcess, INFINITE) != WAIT_FAILED &&
***************
*** 2635,2647 ****
/* Free up the native handle at this point */
CloseHandle(hProcess);
! /* Remove from dictionary and flush dictionary if empty */
! PyDict_DelItem(_PyPopenProcs, fileObj);
! if (PyDict_Size(_PyPopenProcs) == 0) {
! Py_DECREF(_PyPopenProcs);
! _PyPopenProcs = NULL;
! }
! } /* if hProcessObj */
! } /* if fileObj */
} /* if _PyPopenProcs */
--- 2737,2753 ----
/* Free up the native handle at this point */
CloseHandle(hProcess);
+ }
! /* Remove this file pointer from dictionary */
! PyDict_DelItem(_PyPopenProcs, fileObj);
!
! if (PyDict_Size(_PyPopenProcs) == 0) {
! Py_DECREF(_PyPopenProcs);
! _PyPopenProcs = NULL;
! }
!
! } /* if object retrieval ok */
!
! Py_XDECREF(fileObj);
} /* if _PyPopenProcs */