Strange hang problem with ReadFile.
Here is the algorithm:
The idea is to launch a python script from one Win32 GUI application,
then read its std output and store it in a file. Do this until the
script is done.
1. I create a child process using CreateProcess with the code pretty
much copied from http://support.microsoft.com/default.aspx?scid=kb;en-us;190351.
However, instead of creating some special child process I just launch
a python script.
2. Then start reading from the hChildStdOut using ReadFile.
3. The problem is that ReadFile at the end hangs indefinitely waiting
for more data from the child process.
Is there any way to know if the child process is done (or dead) and
stop reading. I tried several methods to do that, however, they
weren't reliable so far.
1. Check if the ReadFile lpNumberOfBytesRead is smaller than
nNumberOfBytesToRead (http://msdn.microsoft.com/en-us/library/
aa365467%28VS.85%29.aspx). This didn't work because at the beginning
sometimes the Python script returns lpNumberOfBytesRead <
nNumberOfBytesToRead. It does some init and it takes time, I suppose.
2. Use GetExitCodeProcess (http://msdn.microsoft.com/en-us/library/
ms683189%28VS.85%29.aspx) on the child process handle. This method
didn't work either. For some reason the GetExitCodeProcess would
indicate that the process is still active.
Please help.
-Ilya.
This is pretty standard practice coding stuff. So show code to see
where you went wrong.
--
IlyaK wrote:
--
HLS
{
// start the child process
HANDLE hChildStdOut;
HANDLE hChildStdIn;
HANDLE hChildStdErr;
HANDLE hChildProc;
if (!launch_redirected_child(m_testCmdLine.c_str(), &hChildStdOut,
&hChildStdIn, &hChildStdErr, &hChildProc)){
printf("Error creating child\n");
return -1;
}
// Wait for the script to finish its busyness,
// getting the output and checking the cancel
// flag
const int iBufferSize = 256;
char * readBuffer = new char[iBufferSize];
memset(readBuffer, 0, iBufferSize);
int numRead = 0;
unsigned long lErrors = 0;
long isEof = 0;
printf("BEGIN Log:\n");
if (m_bDoScriptLog == true) {
while (get_cancel_test() != true) {
if ( (numRead = pipe_read( hChildStdOut, readBuffer,
iBufferSize )) > 0 ) {
printf("Log: %d\n", numRead);
printf("Log: %s\n", string(readBuffer,
numRead).c_str());
GetExitCodeProcess(hChildProc, &lErrors);
printf("Log: ", lErrors);
if (lErrors != STILL_ACTIVE && numRead < iBufferSize)
{
printf("Log: Exit");
break;
}
} else {
// Check if the pipe is broken, then exit,
// Otherwise report error, and aslo exit
printf("Exit on Error\n");
break;
}
//memset(readBuffer, 0, iBufferSize);
}
}
printf("END Log.\n");
delete [] readBuffer;
status = CloseHandle(hChildStdOut);
status = CloseHandle(hChildStdIn);
status = CloseHandle(hChildStdErr);
status = CloseHandle(hChildProc);
}
/**
* The function will set up STARTUPINFO structure, and launch
redirected child.
* It will return the handles to talk to this child process.
* Make sure that the handles are properly closed after use.
* Lifted from http://support.microsoft.com/default.aspx?scid=kb;en-us;190351
*
* @param command
* @param hChildStdOut
* @param hChildStdIn
* @param hChildStdErr
*
* @return 0 - error occured. Use GetLastError to figure out what
happened.
*/
int
launch_redirected_child(const char * command,
HANDLE * hChildStdOut,
HANDLE * hChildStdIn,
HANDLE * hChildStdErr,
HANDLE * hChildProcess)
{
HANDLE hOutputReadTmp,hOutputRead,hOutputWrite;
HANDLE hInputWriteTmp,hInputRead,hInputWrite;
SECURITY_ATTRIBUTES sa;
PROCESS_INFORMATION pi;
STARTUPINFO si;
// Set up the security attributes struct.
sa.nLength= sizeof(SECURITY_ATTRIBUTES);
sa.lpSecurityDescriptor = NULL;
sa.bInheritHandle = TRUE;
// Create the child output pipe - child sends.
if (!CreatePipe(&hOutputReadTmp,&hOutputWrite,&sa,0)) {
return 0;
}
// Create a duplicate of the output write handle for the std error
// write handle. This is necessary in case the child application
// closes one of its std output handles.
if (!DuplicateHandle(GetCurrentProcess(),hOutputWrite,
GetCurrentProcess(),
hChildStdErr,
0,TRUE,
DUPLICATE_SAME_ACCESS)) {
return 0;
}
// Create the child input pipe - child receives.
if (!CreatePipe(&hInputRead,&hInputWriteTmp,&sa,0)) {
return 0;
}
// Create new output read handle and the input write handles. Set
// the Properties to FALSE. Otherwise, the child inherits the
// properties and, as a result, non-closeable handles to the pipes
// are created.
if (!DuplicateHandle(GetCurrentProcess(),hOutputReadTmp,
GetCurrentProcess(),
hChildStdOut, // Address of new handle.
0,FALSE, // Make it uninheritable.
DUPLICATE_SAME_ACCESS)) {
return 0;
}
if (!DuplicateHandle(GetCurrentProcess(),hInputWriteTmp,
GetCurrentProcess(),
hChildStdIn, // Address of new handle.
0,FALSE, // Make it uninheritable.
DUPLICATE_SAME_ACCESS)) {
return 0;
}
// Close inheritable copies of the handles you do not want to be
// inherited.
if (!CloseHandle(hOutputReadTmp)) {
return 0;
}
if (!CloseHandle(hInputWriteTmp)) {
return 0;
}
// Set up the start up info struct.
ZeroMemory(&si,sizeof(STARTUPINFO));
si.cb = sizeof(STARTUPINFO);
si.dwFlags = STARTF_USESTDHANDLES;
si.hStdOutput = hOutputWrite;
si.hStdInput = hInputRead;
si.hStdError = *hChildStdErr;
// Use this if you want to hide the child:
// si.wShowWindow = SW_HIDE;
// Note that dwFlags must include STARTF_USESHOWWINDOW if you want
to
// use the wShowWindow flags.
// Launch the process that you want to redirect (in this case,
// Child.exe). Make sure Child.exe is in the same directory as
// redirect.c launch redirect from a command line to prevent
location
// confusion.
if (!CreateProcess(NULL,(LPCWSTR)(command),
NULL,NULL,TRUE,
0,NULL,NULL,&si,&pi)) {
return 0;
}
// Close any unnecessary handles.
if (!CloseHandle(pi.hThread)) {
return 0;
}
if (hChildProcess != NULL) {
*hChildProcess = pi.hProcess;
} else {
if (!CloseHandle(pi.hProcess)) {
return 0;
}
}
if (!CloseHandle(hOutputWrite)) {
return 0;
}
if (!CloseHandle(hInputRead)) {
return 0;
}
return 1;
}
int
pipe_read (HANDLE fd, void *buffer, size_t count) {
DWORD numRead;
if (!ReadFile (fd, buffer, (DWORD)count, &numRead, NULL))
{ //non overlapped
return -1;
}
return (int) numRead;
}
On Mar 2, 12:40 pm, Hector Santos <sant9...@nospam.gmail.com> wrote:
> You should show code because that first link with redirection logic
> should work for you. However, there are ways do this without using pipes.
>
> This is pretty standard practice coding stuff. So show code to see
> where you went wrong.
>
> --
>
>
>
> IlyaK wrote:
> > Hello:
>
> > Strange hang problem with ReadFile.
> > Here is the algorithm:
>
> > The idea is to launch a python script from one Win32 GUI application,
> > then read its std output and store it in a file. Do this until the
> > script is done.
> > 1. I create a child process using CreateProcess with the code pretty
> > much copied fromhttp://support.microsoft.com/default.aspx?scid=kb;en-us;190351.
If you don't want interactive I/O and just want to capture output, you
really don't need piping. But it will work the same. The loop should
be something like this example console program:
------------- CUT HERE ------------
// File: d:\wc5beta\testspawn4.cpp
// Author: Hector Santos, Santronics Software, Inc.
// Compile: cl testspawn4.cpp /MT
#include <stdio.h>
#include <afx.h>
/******************************************************************
* SendToParent() is called by the redirection logic to that you
* send the data as a message event to your parent window. In this
* simple example we are just displaying it to the screen.
******************************************************************/
void SendToParent(const char *szBuf, const DWORD size)
{
printf("%s",szBuf);
}
/******************************************************************
* BOOL Run(UseDos, szCmd)
*
* Spawn a child process and capture the redirect output.
*
* UseDos = Set TRUE to use OS command interpreter
* cmd = command to run (if DOS command, set UseDos=TRUE)
*
* return TRUE if successful. Otherwise FALSE, read extended error.
******************************************************************/
BOOL Run(const char *szCmd, const BOOL UseDos = FALSE)
{
SECURITY_ATTRIBUTES sa;
ZeroMemory(&sa, sizeof(sa));
sa.nLength = sizeof(sa);
sa.bInheritHandle = TRUE;
HANDLE hOut = INVALID_HANDLE_VALUE;
HANDLE hRedir = INVALID_HANDLE_VALUE;
// Create Temp File for redirection
char szTempPath[MAX_PATH];
char szOutput[MAX_PATH];
GetTempPath(sizeof(szTempPath),szTempPath);
GetTempFileName(szTempPath, "tmp", 0, szOutput);
// setup redirection handles
// output handle must be WRITE mode, share READ
// redirect handle must be READ mode, share WRITE
hOut = CreateFile(szOutput,
GENERIC_WRITE,
FILE_SHARE_READ,
&sa,
CREATE_ALWAYS,
FILE_ATTRIBUTE_TEMPORARY,
0);
if (hOut == INVALID_HANDLE_VALUE) {
return FALSE;
}
hRedir = CreateFile(szOutput,
GENERIC_READ,
FILE_SHARE_WRITE,
0,
OPEN_EXISTING,
0,
0);
if (hRedir == INVALID_HANDLE_VALUE) {
CloseHandle(hOut);
return FALSE;
}
// setup startup info, set std out/err handles
// hide window
STARTUPINFO si;
ZeroMemory(&si, sizeof(si));
si.cb = sizeof(si);
if (hOut != INVALID_HANDLE_VALUE) {
si.dwFlags |= STARTF_USESTDHANDLES;
si.hStdOutput = hOut;
si.hStdError = hOut;
si.wShowWindow = SW_HIDE;
}
// use the current OS comspec for DOS commands, i.e., DIR
char cmd[MAX_PATH] = {0};
if (UseDos) {
strcpy(cmd,getenv("comspec"));
strcat(cmd," /c ");
}
strcat(cmd,szCmd);
printf("-Process: %s\n",cmd);
PROCESS_INFORMATION pi;
ZeroMemory(&pi, sizeof(pi));
DWORD dwFlags = CREATE_NO_WINDOW; // NT/2000 only
if (!CreateProcess(NULL, (char *)cmd,
NULL, NULL, TRUE,
dwFlags,
NULL, NULL, &si, &pi)) {
int err = GetLastError(); // preserve the CreateProcess error
if (hOut != INVALID_HANDLE_VALUE) {
CloseHandle(hOut);
CloseHandle(hRedir);
}
SetLastError(err);
return FALSE;
}
// let go of the child thread
printf("-Waiting\n");
CloseHandle(pi.hThread);
// wait for process to finish, capture redirection and
// send it to the parent window/console
DWORD dw;
char buf[256];
do {
ZeroMemory(&buf,sizeof(buf));
while (ReadFile(hRedir, &buf, sizeof(buf)-1, &dw, NULL)) {
if (dw == 0) break;
SendToParent(buf,dw);
ZeroMemory(&buf,sizeof(buf));
}
} while (WaitForSingleObject(pi.hProcess, 0) != WAIT_OBJECT_0);
// perform any final flushing
while (ReadFile(hRedir, &buf, sizeof(buf)-1, &dw, NULL)) {
if (dw == 0) break;
SendToParent(buf,dw);
ZeroMemory(&buf,sizeof(buf));
}
WaitForSingleObject(pi.hProcess, INFINITE);
CloseHandle(pi.hProcess);
CloseHandle(hOut);
CloseHandle(hRedir);
DeleteFile(szOutput);
printf("-finished\n");
return TRUE;
}
void main(char argc, char *argv[])
{
//------------------------------------------
// example #2 - using direct
//------------------------------------------
if (!Run("dir *.cpp", TRUE)) {
printf("Error %d\n",GetLastError());
return;
}
printf("-done\n");
}
---------------- CUT HERE ----------------
You can add a sanity/cancel check in the loop. Just make sure its not
blocking.
IlyaK wrote:
--
HLS
1. get_cancel_text() is just an abstraction of a higher level function
that checks if the user pressed some button. I know I don't block on
it because2. I stepped through the code with the debugger.
2. If you know a better way to do this without piping, let me know. I
don't need interactive IO.
3. I don't prematurely close the processor handle, if the
hChildProcess != NULL. I do close the main thread handle. Do you think
this is a problem?
4. Your code calls ReadFile until it returns false. I tried that, but
I still get stuck.
5 Does it make any difference that I do all this in mfc application?
Thank you,
-Ilya.
> Thanks Hector.
>
> 1. get_cancel_text() is just an abstraction of a higher level function
> that checks if the user pressed some button. I know I don't block on
> it because2. I stepped through the code with the debugger.
> 2. If you know a better way to do this without piping, let me know. I
> don't need interactive IO.
> 3. I don't prematurely close the processor handle, if the
> hChildProcess != NULL. I do close the main thread handle. Do you think
> this is a problem?
No, you have that right.
> 4. Your code calls ReadFile until it returns false. I tried that, but
> I still get stuck.
> 5 Does it make any difference that I do all this in mfc application?
Depends what you are doing with the message queue and how its called.
This is python? hmmmm, that shouldn't be an issue.
For our web server script map support, the code is very similar and
supports all standard I/O CGI tools.
There are several possible issues and requires:
- The child process must be standard output and
can't be waiting for input.
- If its possible no output, then you need an async
ReadFile (Overlapping I/O) timeout.
Since you said its blocking, then I am guessing the python script is
not returning output and its not quiting.
What is the command you are using?
It worked fine with this simple hello1.py
def greet(name):
print 'hello', name
greet('Jack')
greet('Jill')
greet('Bob')
The code runs:
run("python hello1.py", TRUE);
with the output:
-- Process: F:\WINDOWS\system32\cmd.exe /c python hello1.py
-- waiting
hello Jack
hello Jill
hello Bob
-- finished
-- done
Whats in your get_cancel_text() function?
If you are NOT doing this in multi-threaded fashion, then in your MFC,
when you start it, the queue is not called unless you do it by peeking
into it.
But I didn't even go there, I put the code I posted into a MFC program
with a RUN button, a Edit field and I captured the output into a list
box.
So I suspect maybe your python script perhaps is not returning?
--
HLS
On Mar 2, 9:14 pm, Jonathan de Boyne Pollard <J.deBoynePollard-
I'll a couple of points from your code.
1. Using "cmd /c python.exe code.py". In my code I just start
"python.exe code.py". However, I did notice that if I wait a bit the
GetExitCodeProcess would not return STILL_ACTIVE.
2. get_cancel_text() just returns a flag that was set when user
pressed a button on GUI.
3. You use temporary files instead of pipes. I'll try that.
I did try using _popen() before. But for some reason it didn't work.
It didn't get stuck, just exist right away.
Perhaps that note on MSDN site might cause this problem.
Can you show your get_cancel_test()?
The loop you have does not loo correct. You need to do it in a manner
similar or close to what I showed it. The way you have it can
potentially allow a block read which may be because of the compiler
optimizing the loop.
Try this:
for (;;)
{
if (get_cancel_test() == true) break;
if (WaitForSingleObject(hChildProc, 0) == WAIT_OBJECT_0) break;
if (!GetExitCodeProcess(hChildProc, &lErrors) ||
lErrors != STILL_ACTIVE) break;
if ( (numRead = pipe_read( hChildStdOut,
readBuffer,iBufferSize )) > 0 ) {
printf("Log: %s\n", readBuffer);
} else {
break;
}
}
--
hls
--
HLS
> All that handle duplication came from the Microsoft's example. I
> didn't invent it. If there is a better way, I'll be happy to use it.
>
Cargo cult programming is never the answer. You need to understand why
the example is doing what it is doing. At the very least you need to
read the comments in the example, including the ones that you didn't
copy. Now that I know that you copied this from an example in a MSKB
article, I know what to look for, and I find that the example actually
warns about the error that you are making.
This algorithm is working!
There is, however, one small caveat. It has to check if numRead ==
iBufferSize. If true, then there is more data and it should read again
before exiting.
Thanks for help.
Apply the following:
- The read will return zero when the last write handle to the pipe
closes.
- The child process has completed, so it can't be holding handles.
So who has this handle open?
The main process.
So close *that* copy of the handle and it'll work as expected.
Regards,
Roger.