From ed8cd1269439c076df039cb73e5d91fe09c42a78 Mon Sep 17 00:00:00 2001 From: Michihiro NAKAJIMA Date: Fri, 5 Oct 2012 10:54:38 +0900 Subject: [PATCH] Avoid infinity wait in ReadFile during running an external program as decoding filter on Windows. --- .../archive_read_support_filter_program.c | 51 ++++++++++++++++++- libarchive/archive_windows.c | 39 +++++--------- libarchive/archive_windows.h | 2 +- libarchive/archive_write_add_filter_program.c | 33 ++++++++++-- 4 files changed, 93 insertions(+), 32 deletions(-) diff --git a/libarchive/archive_read_support_filter_program.c b/libarchive/archive_read_support_filter_program.c index 2b5540aba..ae6f3990d 100644 --- a/libarchive/archive_read_support_filter_program.c +++ b/libarchive/archive_read_support_filter_program.c @@ -171,7 +171,11 @@ static int program_bidder_free(struct archive_read_filter_bidder *); */ struct program_filter { struct archive_string description; +#if defined(_WIN32) && !defined(__CYGWIN__) + HANDLE child; +#else pid_t child; +#endif int exit_status; int waitpid_return; int child_stdin, child_stdout; @@ -435,6 +439,9 @@ child_stop(struct archive_read_filter *self, struct program_filter *state) state->waitpid_return = waitpid(state->child, &state->exit_status, 0); } while (state->waitpid_return == -1 && errno == EINTR); +#if defined(_WIN32) && !defined(__CYGWIN__) + CloseHandle(state->child); +#endif state->child = 0; } @@ -487,11 +494,35 @@ child_read(struct archive_read_filter *self, char *buf, size_t buf_len) struct program_filter *state = self->data; ssize_t ret, requested, avail; const char *p; +#if defined(_WIN32) && !defined(__CYGWIN__) + HANDLE handle = (HANDLE)_get_osfhandle(state->child_stdout); +#endif requested = buf_len > SSIZE_MAX ? SSIZE_MAX : buf_len; for (;;) { do { +#if defined(_WIN32) && !defined(__CYGWIN__) + /* Avoid infinity wait. + * Note: If there is no data in the pipe, ReadFile() + * called in read() never returns and so we won't + * write remaining encoded data to the pipe. + * Note: This way may cause performance problem. + * we are looking forward to great code to resolve + * this. */ + DWORD pipe_avail = -1; + int cnt = 2; + + while (PeekNamedPipe(handle, NULL, 0, NULL, + &pipe_avail, NULL) != 0 && pipe_avail == 0 && + cnt--) + Sleep(5); + if (pipe_avail == 0) { + ret = -1; + errno = EAGAIN; + break; + } +#endif ret = read(state->child_stdout, buf, requested); } while (ret == -1 && errno == EINTR); @@ -624,6 +655,7 @@ __archive_read_programv(struct archive_read_filter *self, const char *cmd, static const size_t out_buf_len = 65536; char *out_buf; const char *prefix = "Program: "; + pid_t child; int i; size_t l; @@ -654,8 +686,9 @@ __archive_read_programv(struct archive_read_filter *self, const char *cmd, state->out_buf = out_buf; state->out_buf_len = out_buf_len; - if ((state->child = __archive_create_child(cmd, argv, - &state->child_stdin, &state->child_stdout)) == -1) { + child = __archive_create_child(cmd, argv, &state->child_stdin, + &state->child_stdout); + if (child == -1) { free(state->out_buf); free(state); archive_set_error(&self->archive->archive, EINVAL, @@ -663,6 +696,20 @@ __archive_read_programv(struct archive_read_filter *self, const char *cmd, cmd); return (ARCHIVE_FATAL); } +#if defined(_WIN32) && !defined(__CYGWIN__) + state->child = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, child); + if (state->child == NULL) { + child_stop(self, state); + free(state->out_buf); + free(state); + archive_set_error(&self->archive->archive, EINVAL, + "Can't initialize filter; unable to run program \"%s\"", + cmd); + return (ARCHIVE_FATAL); + } +#else + state->child = child; +#endif self->data = state; self->read = program_filter_read; diff --git a/libarchive/archive_windows.c b/libarchive/archive_windows.c index 12ad07dc2..d3bf758bb 100644 --- a/libarchive/archive_windows.c +++ b/libarchive/archive_windows.c @@ -633,35 +633,22 @@ __la_stat(const char *path, struct stat *st) * This waitpid is limited implementation. */ pid_t -__la_waitpid(pid_t wpid, int *status, int option) +__la_waitpid(HANDLE child, int *status, int option) { - HANDLE child; - DWORD cs, ret; + DWORD cs; (void)option;/* UNUSED */ - *status = 0; - child = OpenProcess(PROCESS_QUERY_INFORMATION | SYNCHRONIZE, FALSE, wpid); - if (child == NULL) { - la_dosmaperr(GetLastError()); - return (-1); - } - ret = WaitForSingleObject(child, INFINITE); - if (ret == WAIT_FAILED) { - CloseHandle(child); - la_dosmaperr(GetLastError()); - return (-1); - } - if (GetExitCodeProcess(child, &cs) == 0) { - CloseHandle(child); - la_dosmaperr(GetLastError()); - return (-1); - } - if (cs == STILL_ACTIVE) - *status = 0x100; - else - *status = (int)(cs & 0xff); - CloseHandle(child); - return (wpid); + do { + if (GetExitCodeProcess(child, &cs) == 0) { + CloseHandle(child); + la_dosmaperr(GetLastError()); + *status = 0; + return (-1); + } + } while (cs == STILL_ACTIVE); + + *status = (int)(cs & 0xff); + return (0); } ssize_t diff --git a/libarchive/archive_windows.h b/libarchive/archive_windows.h index f755d5d72..c6f5bc510 100644 --- a/libarchive/archive_windows.h +++ b/libarchive/archive_windows.h @@ -264,7 +264,7 @@ extern __int64 __la_lseek(int fd, __int64 offset, int whence); extern int __la_open(const char *path, int flags, ...); extern ssize_t __la_read(int fd, void *buf, size_t nbytes); extern int __la_stat(const char *path, struct stat *st); -extern pid_t __la_waitpid(pid_t wpid, int *status, int option); +extern pid_t __la_waitpid(HANDLE child, int *status, int option); extern ssize_t __la_write(int fd, const void *buf, size_t nbytes); #define _stat64i32(path, st) __la_stat(path, st) diff --git a/libarchive/archive_write_add_filter_program.c b/libarchive/archive_write_add_filter_program.c index 7ccc059c1..4533e4da1 100644 --- a/libarchive/archive_write_add_filter_program.c +++ b/libarchive/archive_write_add_filter_program.c @@ -62,7 +62,11 @@ struct private_data { char *cmd; char **argv; struct archive_string description; +#if defined(_WIN32) && !defined(__CYGWIN__) + HANDLE child; +#else pid_t child; +#endif int child_stdin, child_stdout; char *child_buf; @@ -235,6 +239,7 @@ static int archive_compressor_program_open(struct archive_write_filter *f) { struct private_data *data = (struct private_data *)f->data; + pid_t child; int ret; ret = __archive_write_open_filter(f->next_filter); @@ -253,13 +258,27 @@ archive_compressor_program_open(struct archive_write_filter *f) } } - if ((data->child = __archive_create_child(data->cmd, - (char * const *)data->argv, - &data->child_stdin, &data->child_stdout)) == -1) { + child = __archive_create_child(data->cmd, (char * const *)data->argv, + &data->child_stdin, &data->child_stdout); + if (child == -1) { + archive_set_error(f->archive, EINVAL, + "Can't initialise filter"); + return (ARCHIVE_FATAL); + } +#if defined(_WIN32) && !defined(__CYGWIN__) + data->child = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, child); + if (data->child == NULL) { + close(data->child_stdin); + data->child_stdin = -1; + close(data->child_stdout); + data->child_stdout = -1; archive_set_error(f->archive, EINVAL, "Can't initialise filter"); return (ARCHIVE_FATAL); } +#else + data->child = child; +#endif f->write = archive_compressor_program_write; f->close = archive_compressor_program_close; @@ -411,6 +430,10 @@ cleanup: close(data->child_stdout); while (waitpid(data->child, &status, 0) == -1 && errno == EINTR) continue; +#if defined(_WIN32) && !defined(__CYGWIN__) + CloseHandle(data->child); +#endif + data->child = 0; if (status != 0) { archive_set_error(f->archive, EIO, @@ -427,6 +450,10 @@ archive_compressor_program_free(struct archive_write_filter *f) struct private_data *data = (struct private_data *)f->data; if (data) { +#if defined(_WIN32) && !defined(__CYGWIN__) + if (data->child) + CloseHandle(data->child); +#endif if (data->argv != NULL) { int i; for (i = 0; data->argv[i] != NULL; i++) -- 2.47.2