Skip to content

Commit

Permalink
Avoid infinity wait in ReadFile during running an external program as
Browse files Browse the repository at this point in the history
decoding filter on Windows.
  • Loading branch information
ggcueroad committed Oct 5, 2012
1 parent 40722a2 commit ed8cd12
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 32 deletions.
51 changes: 49 additions & 2 deletions libarchive/archive_read_support_filter_program.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -654,15 +686,30 @@ __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,
"Can't initialize filter; unable to run program \"%s\"",
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;
Expand Down
39 changes: 13 additions & 26 deletions libarchive/archive_windows.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion libarchive/archive_windows.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
33 changes: 30 additions & 3 deletions libarchive/archive_write_add_filter_program.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -411,6 +430,10 @@ archive_compressor_program_close(struct archive_write_filter *f)
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,
Expand All @@ -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++)
Expand Down

0 comments on commit ed8cd12

Please sign in to comment.