[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: DuplicateHandle(in) failed (e=6)
|
From: |
Lynn Lin |
|
Subject: |
Re: DuplicateHandle(in) failed (e=6) |
|
Date: |
Sun, 29 Jan 2012 00:57:42 +0800 |
On Sat, Jan 21, 2012 at 11:37 PM, Eli Zaretskii <address@hidden> wrote:
>> Date: Sun, 15 Jan 2012 20:48:58 +0200
>> From: Eli Zaretskii <address@hidden>
>> Cc: address@hidden, address@hidden
>>
>> > Date: Sun, 15 Jan 2012 08:28:46 +0800
>> > From: Lynn Lin <address@hidden>
>> > Cc: address@hidden, address@hidden
>> >
>> > Is that fine you send me a patch to me so I can verify it?
>> > Thanks very much!
>>
>> I will, once I figure out what is the right patch.
>
> Here, can you please try the patch below?
>
Thanks much,I will try this patch and get back here
>
> 2012-01-21 Eli Zaretskii <address@hidden>
>
> Fix failures on MS-Windows when Make's standard handles are invalid.
> * w32/subproc/sub_proc.c (process_init_fd): Don't dereference
> pproc if it is a NULL pointer.
> (process_begin, process_cleanup): Don't try to close pipe handles
> whose value is INVALID_HANDLE_VALUE.
> (process_easy): Initialize hIn, hOut, and hErr to
> INVALID_HANDLE_VALUE. If DuplicateHandle fails with
> ERROR_INVALID_HANDLE, duplicate a handle for the null device
> instead of STD_INPUT_HANDLE, STD_OUTPUT_HANDLE or
> STD_ERROR_HANDLE. Don't try to close pipe handles whose value is
> INVALID_HANDLE_VALUE.
>
> * function.c (windows32_openpipe): Initialize hIn and hErr to
> INVALID_HANDLE_VALUE. If DuplicateHandle fails with
> ERROR_INVALID_HANDLE, duplicate a handle for the null device
> instead of STD_INPUT_HANDLE or STD_ERROR_HANDLE. Fix indentation.
> Don't try to close handles whose value is INVALID_HANDLE_VALUE.
>
>
>
> --- w32/subproc/sub_proc.c~0 2010-07-19 13:10:54.000000000 +0300
> +++ w32/subproc/sub_proc.c 2012-01-21 17:13:40.419250000 +0200
> @@ -336,17 +336,19 @@ process_init_fd(HANDLE stdinh, HANDLE st
> sub_process *pproc;
>
> pproc = malloc(sizeof(*pproc));
> - memset(pproc, 0, sizeof(*pproc));
> + if (pproc) {
> + memset(pproc, 0, sizeof(*pproc));
>
> - /*
> - * Just pass the provided file handles to the 'child side' of the
> - * pipe, bypassing pipes altogether.
> - */
> - pproc->sv_stdin[1] = (intptr_t) stdinh;
> - pproc->sv_stdout[1] = (intptr_t) stdouth;
> - pproc->sv_stderr[1] = (intptr_t) stderrh;
> + /*
> + * Just pass the provided file handles to the 'child
> + * side' of the pipe, bypassing pipes altogether.
> + */
> + pproc->sv_stdin[1] = (intptr_t) stdinh;
> + pproc->sv_stdout[1] = (intptr_t) stdouth;
> + pproc->sv_stderr[1] = (intptr_t) stderrh;
>
> - pproc->last_err = pproc->lerrno = 0;
> + pproc->last_err = pproc->lerrno = 0;
> + }
>
> return((HANDLE)pproc);
> }
> @@ -585,9 +587,12 @@ process_begin(
> CloseHandle(procInfo.hThread);
>
> /* Close the halves of the pipes we don't need */
> - CloseHandle((HANDLE)pproc->sv_stdin[1]);
> - CloseHandle((HANDLE)pproc->sv_stdout[1]);
> - CloseHandle((HANDLE)pproc->sv_stderr[1]);
> + if ((HANDLE)pproc->sv_stdin[1] != INVALID_HANDLE_VALUE)
> + CloseHandle((HANDLE)pproc->sv_stdin[1]);
> + if ((HANDLE)pproc->sv_stdout[1] != INVALID_HANDLE_VALUE)
> + CloseHandle((HANDLE)pproc->sv_stdout[1]);
> + if ((HANDLE)pproc->sv_stderr[1] != INVALID_HANDLE_VALUE)
> + CloseHandle((HANDLE)pproc->sv_stderr[1]);
> pproc->sv_stdin[1] = 0;
> pproc->sv_stdout[1] = 0;
> pproc->sv_stderr[1] = 0;
> @@ -938,11 +943,14 @@ process_cleanup(
>
> if (pproc->using_pipes) {
> for (i= 0; i <= 1; i++) {
> - if ((HANDLE)pproc->sv_stdin[i])
> + if ((HANDLE)pproc->sv_stdin[i]
> + && (HANDLE)pproc->sv_stdin[i] !=
> INVALID_HANDLE_VALUE)
> CloseHandle((HANDLE)pproc->sv_stdin[i]);
> - if ((HANDLE)pproc->sv_stdout[i])
> + if ((HANDLE)pproc->sv_stdout[i]
> + && (HANDLE)pproc->sv_stdout[i] !=
> INVALID_HANDLE_VALUE)
> CloseHandle((HANDLE)pproc->sv_stdout[i]);
> - if ((HANDLE)pproc->sv_stderr[i])
> + if ((HANDLE)pproc->sv_stderr[i]
> + && (HANDLE)pproc->sv_stderr[i] !=
> INVALID_HANDLE_VALUE)
> CloseHandle((HANDLE)pproc->sv_stderr[i]);
> }
> }
> @@ -1207,50 +1215,100 @@ process_easy(
> char **argv,
> char **envp)
> {
> - HANDLE hIn;
> - HANDLE hOut;
> - HANDLE hErr;
> - HANDLE hProcess;
> + HANDLE hIn = INVALID_HANDLE_VALUE;
> + HANDLE hOut = INVALID_HANDLE_VALUE;
> + HANDLE hErr = INVALID_HANDLE_VALUE;
> + HANDLE hProcess, tmpIn, tmpOut, tmpErr;
> + DWORD e;
>
> if (proc_index >= MAXIMUM_WAIT_OBJECTS) {
> DB (DB_JOBS, ("process_easy: All process slots used up\n"));
> return INVALID_HANDLE_VALUE;
> }
> + /* Standard handles returned by GetStdHandle can be NULL or
> + INVALID_HANDLE_VALUE if the parent process closed them. If that
> + happens, we open the null device and pass its handle to
> + CreateProcess as the corresponding handle to inherit. */
> + tmpIn = GetStdHandle(STD_INPUT_HANDLE);
> if (DuplicateHandle(GetCurrentProcess(),
> - GetStdHandle(STD_INPUT_HANDLE),
> - GetCurrentProcess(),
> - &hIn,
> - 0,
> - TRUE,
> - DUPLICATE_SAME_ACCESS) == FALSE) {
> - fprintf(stderr,
> - "process_easy: DuplicateHandle(In) failed (e=%ld)\n",
> - GetLastError());
> - return INVALID_HANDLE_VALUE;
> + tmpIn,
> + GetCurrentProcess(),
> + &hIn,
> + 0,
> + TRUE,
> + DUPLICATE_SAME_ACCESS) == FALSE) {
> + if ((e = GetLastError()) == ERROR_INVALID_HANDLE) {
> + tmpIn = CreateFile("NUL", GENERIC_READ,
> + FILE_SHARE_READ | FILE_SHARE_WRITE, NULL,
> + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> + if (tmpIn != INVALID_HANDLE_VALUE
> + && DuplicateHandle(GetCurrentProcess(),
> + tmpIn,
> + GetCurrentProcess(),
> + &hIn,
> + 0,
> + TRUE,
> + DUPLICATE_SAME_ACCESS) == FALSE)
> + CloseHandle(tmpIn);
> + }
> + if (hIn == INVALID_HANDLE_VALUE) {
> + fprintf(stderr, "process_easy: DuplicateHandle(In) failed (e=%ld)\n",
> e);
> + return INVALID_HANDLE_VALUE;
> + }
> }
> + tmpOut = GetStdHandle (STD_OUTPUT_HANDLE);
> if (DuplicateHandle(GetCurrentProcess(),
> - GetStdHandle(STD_OUTPUT_HANDLE),
> - GetCurrentProcess(),
> - &hOut,
> - 0,
> - TRUE,
> - DUPLICATE_SAME_ACCESS) == FALSE) {
> - fprintf(stderr,
> - "process_easy: DuplicateHandle(Out) failed (e=%ld)\n",
> - GetLastError());
> - return INVALID_HANDLE_VALUE;
> + tmpOut,
> + GetCurrentProcess(),
> + &hOut,
> + 0,
> + TRUE,
> + DUPLICATE_SAME_ACCESS) == FALSE) {
> + if ((e = GetLastError()) == ERROR_INVALID_HANDLE) {
> + tmpOut = CreateFile("NUL", GENERIC_WRITE,
> + FILE_SHARE_READ | FILE_SHARE_WRITE, NULL,
> + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> + if (tmpOut != INVALID_HANDLE_VALUE
> + && DuplicateHandle(GetCurrentProcess(),
> + tmpOut,
> + GetCurrentProcess(),
> + &hOut,
> + 0,
> + TRUE,
> + DUPLICATE_SAME_ACCESS) == FALSE)
> + CloseHandle(tmpOut);
> + }
> + if (hOut == INVALID_HANDLE_VALUE) {
> + fprintf(stderr, "process_easy: DuplicateHandle(Out) failed (e=%ld)\n",
> e);
> + return INVALID_HANDLE_VALUE;
> + }
> }
> + tmpErr = GetStdHandle(STD_ERROR_HANDLE);
> if (DuplicateHandle(GetCurrentProcess(),
> - GetStdHandle(STD_ERROR_HANDLE),
> - GetCurrentProcess(),
> - &hErr,
> - 0,
> - TRUE,
> - DUPLICATE_SAME_ACCESS) == FALSE) {
> - fprintf(stderr,
> - "process_easy: DuplicateHandle(Err) failed (e=%ld)\n",
> - GetLastError());
> - return INVALID_HANDLE_VALUE;
> + tmpErr,
> + GetCurrentProcess(),
> + &hErr,
> + 0,
> + TRUE,
> + DUPLICATE_SAME_ACCESS) == FALSE) {
> + if ((e = GetLastError()) == ERROR_INVALID_HANDLE) {
> + tmpErr = CreateFile("NUL", GENERIC_WRITE,
> + FILE_SHARE_READ | FILE_SHARE_WRITE, NULL,
> + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> + if (tmpErr != INVALID_HANDLE_VALUE
> + && DuplicateHandle(GetCurrentProcess(),
> + tmpErr,
> + GetCurrentProcess(),
> + &hErr,
> + 0,
> + TRUE,
> + DUPLICATE_SAME_ACCESS) == FALSE)
> + CloseHandle(tmpErr);
> + }
> + if (hErr == INVALID_HANDLE_VALUE) {
> + fprintf(stderr, "process_easy: DuplicateHandle(Err) failed (e=%ld)\n",
> e);
> + return INVALID_HANDLE_VALUE;
> + }
> }
>
> hProcess = process_init_fd(hIn, hOut, hErr);
> @@ -1263,9 +1321,12 @@ process_easy(
> ((sub_process*) hProcess)->exit_code = process_last_err(hProcess);
>
> /* close up unused handles */
> - CloseHandle(hIn);
> - CloseHandle(hOut);
> - CloseHandle(hErr);
> + if (hIn != INVALID_HANDLE_VALUE)
> + CloseHandle(hIn);
> + if (hOut != INVALID_HANDLE_VALUE)
> + CloseHandle(hOut);
> + if (hErr != INVALID_HANDLE_VALUE)
> + CloseHandle(hErr);
> }
>
> process_register(hProcess);
> --- function.c~0 2010-08-07 11:27:11.981125000 +0300
> +++ function.c 2012-01-21 17:23:21.684875000 +0200
> @@ -1437,37 +1437,70 @@ void
> windows32_openpipe (int *pipedes, pid_t *pid_p, char **command_argv, char
> **envp)
> {
> SECURITY_ATTRIBUTES saAttr;
> - HANDLE hIn;
> - HANDLE hErr;
> + HANDLE hIn = INVALID_HANDLE_VALUE;
> + HANDLE hErr = INVALID_HANDLE_VALUE;
> HANDLE hChildOutRd;
> HANDLE hChildOutWr;
> - HANDLE hProcess;
> -
> + HANDLE hProcess, tmpIn, tmpErr;
> + DWORD e;
>
> saAttr.nLength = sizeof (SECURITY_ATTRIBUTES);
> saAttr.bInheritHandle = TRUE;
> saAttr.lpSecurityDescriptor = NULL;
>
> + /* Standard handles returned by GetStdHandle can be NULL or
> + INVALID_HANDLE_VALUE if the parent process closed them. If that
> + happens, we open the null device and pass its handle to
> + process_begin below as the corresponding handle to inherit. */
> + tmpIn = GetStdHandle(STD_INPUT_HANDLE);
> if (DuplicateHandle (GetCurrentProcess(),
> - GetStdHandle(STD_INPUT_HANDLE),
> + tmpIn,
> GetCurrentProcess(),
> &hIn,
> 0,
> TRUE,
> DUPLICATE_SAME_ACCESS) == FALSE) {
> - fatal (NILF, _("windows32_openpipe(): DuplicateHandle(In) failed
> (e=%ld)\n"),
> - GetLastError());
> -
> + if ((e = GetLastError()) == ERROR_INVALID_HANDLE) {
> + tmpIn = CreateFile("NUL", GENERIC_READ,
> + FILE_SHARE_READ | FILE_SHARE_WRITE, NULL,
> + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> + if (tmpIn != INVALID_HANDLE_VALUE
> + && DuplicateHandle(GetCurrentProcess(),
> + tmpIn,
> + GetCurrentProcess(),
> + &hIn,
> + 0,
> + TRUE,
> + DUPLICATE_SAME_ACCESS) == FALSE)
> + CloseHandle(tmpIn);
> + }
> + if (hIn == INVALID_HANDLE_VALUE)
> + fatal (NILF, _("windows32_openpipe: DuplicateHandle(In) failed
> (e=%ld)\n"), e);
> }
> + tmpErr = GetStdHandle(STD_ERROR_HANDLE);
> if (DuplicateHandle(GetCurrentProcess(),
> - GetStdHandle(STD_ERROR_HANDLE),
> + tmpErr,
> GetCurrentProcess(),
> &hErr,
> 0,
> TRUE,
> DUPLICATE_SAME_ACCESS) == FALSE) {
> - fatal (NILF, _("windows32_open_pipe(): DuplicateHandle(Err) failed
> (e=%ld)\n"),
> - GetLastError());
> + if ((e = GetLastError()) == ERROR_INVALID_HANDLE) {
> + tmpErr = CreateFile("NUL", GENERIC_WRITE,
> + FILE_SHARE_READ | FILE_SHARE_WRITE, NULL,
> + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> + if (tmpErr != INVALID_HANDLE_VALUE
> + && DuplicateHandle(GetCurrentProcess(),
> + tmpErr,
> + GetCurrentProcess(),
> + &hErr,
> + 0,
> + TRUE,
> + DUPLICATE_SAME_ACCESS) == FALSE)
> + CloseHandle(tmpErr);
> + }
> + if (hErr == INVALID_HANDLE_VALUE)
> + fatal (NILF, _("windows32_openpipe: DuplicateHandle(Err) failed
> (e=%ld)\n"), e);
> }
>
> if (!CreatePipe(&hChildOutRd, &hChildOutWr, &saAttr, 0))
> @@ -1486,23 +1519,25 @@ windows32_openpipe (int *pipedes, pid_t
>
> if (!process_begin(hProcess, command_argv, envp, command_argv[0], NULL)) {
> /* register process for wait */
> - process_register(hProcess);
> + process_register(hProcess);
>
> /* set the pid for returning to caller */
> - *pid_p = (pid_t) hProcess;
> + *pid_p = (pid_t) hProcess;
>
> - /* set up to read data from child */
> - pipedes[0] = _open_osfhandle((intptr_t) hChildOutRd, O_RDONLY);
> + /* set up to read data from child */
> + pipedes[0] = _open_osfhandle((intptr_t) hChildOutRd, O_RDONLY);
>
> - /* this will be closed almost right away */
> - pipedes[1] = _open_osfhandle((intptr_t) hChildOutWr, O_APPEND);
> + /* this will be closed almost right away */
> + pipedes[1] = _open_osfhandle((intptr_t) hChildOutWr, O_APPEND);
> } else {
> /* reap/cleanup the failed process */
> process_cleanup(hProcess);
>
> /* close handles which were duplicated, they weren't used */
> - CloseHandle(hIn);
> - CloseHandle(hErr);
> + if (hIn != INVALID_HANDLE_VALUE)
> + CloseHandle(hIn);
> + if (hErr != INVALID_HANDLE_VALUE)
> + CloseHandle(hErr);
>
> /* close pipe handles, they won't be used */
> CloseHandle(hChildOutRd);