Avoid infinity wait in ReadFile during running an external program as

decoding filter on Windows.

diff --git a/libarchive/archive_read_support_filter_program.c b/libarchive/archive_read_support_filter_program.c
index 2b5540abab46ef96d1f1f27e9cf20e5f4ca81f35..ae6f3990d281e4cd3b72897fb6004aad3308b8ff 100644 (file)
--- 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 12ad07dc2a741474bc5730d1e6c17b1e95824d5c..d3bf758bb39efe38d9c381c57af5b8811f61e541 100644 (file)
--- 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 f755d5d72ae806504af664a030037857fafd2e21..c6f5bc51051353aa735225aaef0d03af550ed5b7 100644 (file)
--- 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 7ccc059c198d2a6536b22920416dfcbd641b9452..4533e4da151d58b1ffd823d757edbb41b7f8447a 100644 (file)
--- 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++)