| From 2a755d2ce77147c6319d18e7b155a7278398f35b Mon Sep 17 00:00:00 2001 |
| From: Hans Beckerus <hans.beckerus at gmail.com> |
| Date: Mon, 9 Aug 2021 21:08:18 +0200 |
| Subject: [PATCH] Make sure reader thread is terminated properly |
| |
| Under some conditions the reader thread responsible for carrying data |
| from the child process to the I/O buffer may fail to terminate at close |
| and file system thread gets blocked waiting for join to complete. |
| Terminate the reader thread using a control message rather then using |
| the brute force call to pthread_cancel(). |
| |
| Additional updates made in this patch to increase robustness include: |
| - use of unbuffered- rather than buffered read on pipe |
| - increase size of pipe on systems that allows it |
| - do not try to promote rdlock to wrlock due to the potential race |
| condition |
| - remove some unnecessary initializations |
| - improve code in I/O buffer to make it more readable |
| - set default debug level to 0 (in case of --enable-debug) |
| - use a per session mutex in I/O buffer rather than a global one |
| - perform dry-run extraction outside of child process and before it |
| is spawned |
| |
| Note that the majority of the changes done as part of this patch only |
| affects extraction of compressed (-m1 and above) and/or encrypted archives. |
| |
| Resolves-issue: #165 |
| Signed-off-by: Hans Beckerus <hans.beckerus at gmail.com> |
| --- |
| configure.ac | 4 +- |
| src/debug.h | 4 +- |
| src/filecache.h | 4 +- |
| src/iobuffer.c | 111 ++++++++++------ |
| src/iobuffer.h | 25 ++-- |
| src/optdb.c | 2 +- |
| src/rar2fs.c | 337 +++++++++++++++++++++++------------------------- |
| 7 files changed, 257 insertions(+), 230 deletions(-) |
| |
| diff --git a/configure.ac b/configure.ac |
| index bee9255..805be18 100644 |
| --- a/configure.ac |
| +++ b/configure.ac |
| @@ -264,11 +264,11 @@ AC_SUBST(UNRAR_LDFLAGS) |
| enableval= |
| AC_ARG_ENABLE([debug], |
| [AS_HELP_STRING([--enable-debug[[=LEVEL]]], |
| - [enable debug mode [LEVEL=4]])], |
| + [enable debug mode [LEVEL=0]])], |
| [], []) |
| if test x"$enableval" != x"no"; then |
| if test x"$enableval" = x"yes"; then |
| - debug_level=4 |
| + debug_level=0 |
| else |
| debug_level=$enableval |
| fi |
| diff --git a/src/debug.h b/src/debug.h |
| index 30e5ae1..ad9200f 100644 |
| --- a/src/debug.h |
| +++ b/src/debug.h |
| @@ -36,7 +36,7 @@ |
| #define DEBUG_ 5 |
| #endif |
| |
| -#if defined ( DEBUG_ ) |
| +#if DEBUG_ > 0 |
| #if DEBUG_ > 2 |
| #define ELVL_ 2 |
| #else |
| @@ -51,7 +51,7 @@ |
| #define ENTER_(...) |
| #endif |
| |
| -#ifdef DEBUG_ |
| +#if DEBUG_ > 0 |
| #define printd(l, fmt, ...) \ |
| do{ \ |
| if (l <= DEBUG_) \ |
| diff --git a/src/filecache.h b/src/filecache.h |
| index d413496..b0026b0 100644 |
| --- a/src/filecache.h |
| +++ b/src/filecache.h |
| @@ -61,7 +61,7 @@ struct filecache_entry { |
| unsigned int vsize_resolved:1; |
| unsigned int :21; |
| unsigned int unresolved:1; |
| - unsigned int :1; |
| + unsigned int dry_run_done:1; |
| unsigned int check_atime:1; |
| unsigned int direct_io:1; |
| unsigned int avi_tested:1; |
| @@ -71,7 +71,7 @@ struct filecache_entry { |
| unsigned int avi_tested:1; |
| unsigned int direct_io:1; |
| unsigned int check_atime:1; |
| - unsigned int :1; |
| + unsigned int dry_run_done:1; |
| unsigned int unresolved:1; |
| unsigned int :21; |
| unsigned int vsize_resolved:1; |
| diff --git a/src/iobuffer.c b/src/iobuffer.c |
| index d81f99d..b191f94 100644 |
| --- a/src/iobuffer.c |
| +++ b/src/iobuffer.c |
| @@ -36,35 +36,31 @@ |
| size_t iob_hist_sz = 0; |
| size_t iob_sz = 0; |
| |
| -static pthread_mutex_t io_mutex = PTHREAD_MUTEX_INITIALIZER; |
| - |
| -#define SPACE_LEFT(ri, wi) (IOB_SZ - SPACE_USED((ri), (wi))) |
| +/* -1 to avoid wi = ri */ |
| +#define SPACE_LEFT(ri, wi) ((IOB_SZ - SPACE_USED((ri), (wi))) - IOB_HIST_SZ - 1) |
| #define SPACE_USED(ri, wi) (((wi) - (ri)) & (IOB_SZ-1)) |
| |
| /*! |
| ***************************************************************************** |
| * |
| ****************************************************************************/ |
| -size_t iob_write(struct iob *dest, FILE *fp, int hist) |
| +size_t iob_write(struct iob *iob, int fd) |
| { |
| unsigned tot = 0; |
| - pthread_mutex_lock(&io_mutex); |
| - unsigned int lwi = dest->wi; /* read once */ |
| - unsigned int lri = dest->ri; |
| - pthread_mutex_unlock(&io_mutex); |
| - size_t left = SPACE_LEFT(lri, lwi) - 1; /* -1 to avoid wi = ri */ |
| - if (IOB_HIST_SZ && hist == IOB_SAVE_HIST) { |
| - left = left > IOB_HIST_SZ ? left - IOB_HIST_SZ : 0; |
| - if (!left) { |
| - return 0; /* quick exit */ |
| - } |
| - } |
| + pthread_mutex_lock(&iob->lock); |
| + unsigned int lwi = iob->wi; /* read once */ |
| + unsigned int lri = iob->ri; |
| + pthread_mutex_unlock(&iob->lock); |
| + size_t left = SPACE_LEFT(lri, lwi); |
| + if (!left) |
| + return 0; /* quick exit */ |
| + |
| unsigned int chunk = IOB_SZ - lwi; /* assume one large chunk */ |
| chunk = chunk < left ? chunk : left; /* reconsider assumption */ |
| while (left > 0) { |
| - size_t n = fread(dest->data_p + lwi, 1, chunk, fp); |
| + ssize_t n = read(fd, iob->data_p + lwi, chunk); |
| if (n != chunk) { |
| - if (ferror(fp)) { |
| + if (n < 0) { |
| perror("read"); |
| break; |
| } |
| @@ -77,12 +73,12 @@ size_t iob_write(struct iob *dest, FILE *fp, int hist) |
| chunk -= n; |
| chunk = !chunk ? left : chunk; |
| } |
| - pthread_mutex_lock(&io_mutex); |
| - dest->wi = lwi; |
| - dest->used = SPACE_USED(dest->ri, lwi); /* dest->ri might have changed */ |
| - pthread_mutex_unlock(&io_mutex); |
| + pthread_mutex_lock(&iob->lock); |
| + iob->wi = lwi; |
| + iob->used = SPACE_USED(iob->ri, lwi); /* iob->ri might have changed */ |
| + pthread_mutex_unlock(&iob->lock); |
| MB(); |
| - dest->offset += tot; |
| + iob->offset += tot; |
| |
| return tot; |
| } |
| @@ -91,13 +87,13 @@ size_t iob_write(struct iob *dest, FILE *fp, int hist) |
| ***************************************************************************** |
| * |
| ****************************************************************************/ |
| -size_t iob_read(char *dest, struct iob *src, size_t size, size_t off) |
| +size_t iob_read(char *dest, struct iob *iob, size_t size, size_t off) |
| { |
| size_t tot = 0; |
| - pthread_mutex_lock(&io_mutex); |
| - unsigned int lri = src->ri; /* read once */ |
| - size_t used = src->used; |
| - pthread_mutex_unlock(&io_mutex); |
| + pthread_mutex_lock(&iob->lock); |
| + unsigned int lri = iob->ri; /* read once */ |
| + size_t used = iob->used; |
| + pthread_mutex_unlock(&iob->lock); |
| if (off) { |
| /* consume offset */ |
| off = off < used ? off : used; |
| @@ -108,17 +104,17 @@ size_t iob_read(char *dest, struct iob *src, size_t size, size_t off) |
| unsigned int chunk = IOB_SZ - lri; /* assume one large chunk */ |
| chunk = chunk < size ? chunk : size; /* reconsider assumption */ |
| while (size) { |
| - memcpy(dest, src->data_p + lri, chunk); |
| + memcpy(dest, iob->data_p + lri, chunk); |
| lri = (lri + chunk) & (IOB_SZ - 1); |
| tot += chunk; |
| size -= chunk; |
| dest += chunk; |
| chunk = size; |
| } |
| - pthread_mutex_lock(&io_mutex); |
| - src->ri = lri; |
| - src->used -= tot; /* src->used might have changed */ |
| - pthread_mutex_unlock(&io_mutex); |
| + pthread_mutex_lock(&iob->lock); |
| + iob->ri = lri; |
| + iob->used -= tot; /* iob->used might have changed */ |
| + pthread_mutex_unlock(&iob->lock); |
| |
| return tot; |
| } |
| @@ -127,21 +123,21 @@ size_t iob_read(char *dest, struct iob *src, size_t size, size_t off) |
| ***************************************************************************** |
| * |
| ****************************************************************************/ |
| -size_t iob_copy(char *dest, struct iob *src, size_t size, size_t pos) |
| +size_t iob_copy(char *dest, struct iob *iob, size_t size, size_t pos) |
| { |
| size_t tot = 0; |
| unsigned int chunk = IOB_SZ - pos; /* assume one large chunk */ |
| chunk = chunk < size ? chunk : size; /* reconsider assumption */ |
| - pthread_mutex_lock(&io_mutex); |
| + pthread_mutex_lock(&iob->lock); |
| while (size) { |
| - memcpy(dest, src->data_p + pos, chunk); |
| + memcpy(dest, iob->data_p + pos, chunk); |
| pos = (pos + chunk) & (IOB_SZ - 1); |
| tot += chunk; |
| size -= chunk; |
| dest += chunk; |
| chunk = size; |
| } |
| - pthread_mutex_unlock(&io_mutex); |
| + pthread_mutex_unlock(&iob->lock); |
| return tot; |
| } |
| |
| @@ -165,3 +161,46 @@ void iob_destroy() |
| { |
| } |
| |
| +/*! |
| + ***************************************************************************** |
| + * |
| + ****************************************************************************/ |
| +struct iob *iob_alloc(size_t size) |
| +{ |
| + struct iob *iob; |
| + |
| + iob = calloc(1, size); |
| + if (!iob) |
| + return NULL; |
| + |
| + pthread_mutex_init(&iob->lock, NULL); |
| + pthread_mutex_init(&iob->io_lock, NULL); |
| + |
| + return iob; |
| +} |
| + |
| +/*! |
| + ***************************************************************************** |
| + * |
| + ****************************************************************************/ |
| +void iob_free(struct iob *iob) |
| +{ |
| + if (iob) { |
| + pthread_mutex_destroy(&iob->lock); |
| + pthread_mutex_destroy(&iob->io_lock); |
| + free(iob); |
| + } |
| +} |
| + |
| +/*! |
| + ***************************************************************************** |
| + * |
| + ****************************************************************************/ |
| +int iob_full(struct iob *iob) |
| +{ |
| + int res; |
| + pthread_mutex_lock(&iob->lock); |
| + res = !SPACE_LEFT(iob->ri, iob->wi); |
| + pthread_mutex_unlock(&iob->lock); |
| + return res; |
| +} |
| diff --git a/src/iobuffer.h b/src/iobuffer.h |
| index cdcd3ab..2558361 100644 |
| --- a/src/iobuffer.h |
| +++ b/src/iobuffer.h |
| @@ -35,17 +35,12 @@ |
| #define IOB_SZ_DEFAULT (4 * 1024 * 1024) |
| #ifdef USE_STATIC_IOB_ |
| #define IOB_SZ IOB_SZ_DEFAULT |
| -#define IOB_HIST_SZ (IOB_SZ/2) |
| +#define IOB_HIST_SZ (IOB_SZ / 2) |
| #else |
| #define IOB_SZ (iob_sz) |
| #define IOB_HIST_SZ (iob_hist_sz) |
| #endif |
| |
| -#define IOB_NO_HIST 0 |
| -#define IOB_SAVE_HIST 1 |
| - |
| -#define IOB_RST(b) (memset((b), 0, sizeof(struct iob) + IOB_SZ)) |
| - |
| struct idx_info { |
| int fd; |
| int mmap; |
| @@ -53,22 +48,25 @@ struct idx_info { |
| }; |
| |
| struct iob { |
| + int fd; |
| struct idx_info idx; |
| off_t offset; |
| volatile size_t ri; |
| volatile size_t wi; |
| size_t used; |
| + pthread_mutex_t lock; |
| + pthread_mutex_t io_lock; |
| uint8_t data_p[]; |
| }; |
| |
| size_t |
| -iob_write(struct iob *dest, FILE *fp, int hist); |
| +iob_write(struct iob *iob, int fd); |
| |
| size_t |
| -iob_read(char *dest, struct iob *src, size_t size, size_t off); |
| +iob_read(char *dest, struct iob *iob, size_t size, size_t off); |
| |
| size_t |
| -iob_copy(char *dest, struct iob *src, size_t size, size_t pos); |
| +iob_copy(char *dest, struct iob *iob, size_t size, size_t pos); |
| |
| extern size_t iob_hist_sz; |
| extern size_t iob_sz; |
| @@ -79,5 +77,14 @@ iob_init(); |
| void |
| iob_destroy(); |
| |
| +struct iob * |
| +iob_alloc(size_t size); |
| + |
| +void |
| +iob_free(struct iob *iob); |
| + |
| +int |
| +iob_full(struct iob *iob); |
| + |
| #endif |
| |
| diff --git a/src/optdb.c b/src/optdb.c |
| index f86f5d4..51e63a6 100644 |
| --- a/src/optdb.c |
| +++ b/src/optdb.c |
| @@ -174,7 +174,7 @@ int optdb_save(int opt, const char *s) |
| if (s1) |
| free(s1); |
| |
| -#ifdef DEBUG_ |
| +#if DEBUG_ > 0 |
| { |
| int i; |
| printd(5, "option %d : ", opt); |
| diff --git a/src/rar2fs.c b/src/rar2fs.c |
| index 1d97d8a..3703206 100644 |
| --- a/src/rar2fs.c |
| +++ b/src/rar2fs.c |
| @@ -78,10 +78,10 @@ |
| #define MOUNT_ARCHIVE 1 |
| |
| #define RD_IDLE 0 |
| -#define RD_WAKEUP 1 |
| +#define RD_TERM 1 |
| #define RD_SYNC_NOREAD 2 |
| #define RD_SYNC_READ 3 |
| -#define RD_READ_ASYNC 4 |
| +#define RD_ASYNC_READ 4 |
| |
| /*#define DEBUG_READ*/ |
| |
| @@ -92,6 +92,7 @@ struct volume_handle { |
| |
| struct io_context { |
| FILE* fp; |
| + int pfd[2]; |
| off_t pos; |
| struct iob *buf; |
| pid_t pid; |
| @@ -127,7 +128,6 @@ struct io_handle { |
| char *path; /* type = all */ |
| }; |
| |
| -#define FH_ZERO(fh) ((fh) = 0) |
| #define FH_ISSET(fh) (fh) |
| #define FH_SETCONTEXT(fh, v) (FH_TOIO(fh)->u.context = (v)) |
| #define FH_SETFD(fh, v) (FH_TOIO(fh)->u.fd = (v)) |
| @@ -393,8 +393,10 @@ static int __wait_thread(struct io_context *op) |
| static int __wake_thread(struct io_context *op, int req) |
| { |
| pthread_mutex_lock(&op->rd_req_mutex); |
| - while (op->rd_req) /* sync */ |
| - pthread_cond_wait(&op->rd_req_cond, &op->rd_req_mutex); |
| + if (req != RD_ASYNC_READ) { |
| + while (op->rd_req) /* sync */ |
| + pthread_cond_wait(&op->rd_req_cond, &op->rd_req_mutex); |
| + } |
| op->rd_req = req; |
| pthread_cond_signal(&op->rd_req_cond); |
| pthread_mutex_unlock(&op->rd_req_mutex); |
| @@ -637,8 +639,7 @@ static struct filecache_entry *path_lookup_miss(const char *path, struct stat *s |
| |
| /*! |
| ***************************************************************************** |
| - * This function must always be called with an aquired rdlock but never |
| - * a wrlock. It is however possible that the rdlock is promoted to a wrlock. |
| + * This function must always be called with an aquired wrlock. |
| ****************************************************************************/ |
| static struct filecache_entry *path_lookup(const char *path, struct stat *stbuf) |
| { |
| @@ -652,8 +653,6 @@ static struct filecache_entry *path_lookup(const char *path, struct stat *stbuf) |
| e_p = path_lookup_miss(path, stbuf); |
| if (!e_p) { |
| if (e2_p && e2_p->flags.unresolved) { |
| - pthread_rwlock_unlock(&file_access_lock); |
| - pthread_rwlock_wrlock(&file_access_lock); |
| e2_p->flags.unresolved = 0; |
| if (stbuf) |
| memcpy(stbuf, &e2_p->stat, sizeof(struct stat)); |
| @@ -701,33 +700,40 @@ static int __stop_child(pid_t pid) |
| ***************************************************************************** |
| * |
| ****************************************************************************/ |
| -static FILE *popen_(const struct filecache_entry *entry_p, pid_t *cpid) |
| +static int popen_(struct io_context *op, pid_t *cpid, int *pfd) |
| { |
| - int fd = -1; |
| - int pfd[2] = {-1,}; |
| - |
| pid_t pid; |
| + int ret; |
| + struct filecache_entry *entry_p = op->entry_p; |
| + |
| + pfd[0] = -1; |
| + pfd[1] = -1; |
| + |
| + /* For folder mounts we need to perform an additional dummy |
| + * extraction attempt to avoid feeding the file descriptor |
| + * with garbage data in case of wrong password or CRC errors. */ |
| + if (!entry_p->flags.dry_run_done && mount_type == MOUNT_FOLDER) { |
| + ret = extract_rar(entry_p->rar_p, entry_p->file_p, NULL); |
| + if (ret && ret != ERAR_UNKNOWN) |
| + goto error; |
| + entry_p->flags.dry_run_done = 1; |
| + } |
| + |
| if (pipe(pfd) == -1) { |
| perror("pipe"); |
| goto error; |
| } |
| +#ifdef F_SETPIPE_SZ |
| + /* Increase pipe capacity to some common limit when requested from an |
| + * unprivileged process. */ |
| + fcntl(pfd[0], F_SETPIPE_SZ, 1000000); |
| +#endif |
| |
| pid = fork(); |
| if (pid == 0) { |
| - int ret = 0; |
| setpgid(getpid(), 0); |
| close(pfd[0]); /* Close unused read end */ |
| - /* For folder mounts we need to perform an additional dummy |
| - * extraction attempt to avoid feeding the file descriptor |
| - * with garbage data in case of wrong password or CRC errors. */ |
| - if (mount_type == MOUNT_FOLDER) |
| - ret = extract_rar(entry_p->rar_p, |
| - entry_p->file_p, |
| - NULL); |
| - if (!ret || ret == ERAR_UNKNOWN) |
| - ret = extract_rar(entry_p->rar_p, |
| - entry_p->file_p, |
| - (void *)(uintptr_t)pfd[1]); |
| + ret = extract_rar(entry_p->rar_p, entry_p->file_p, op); |
| close(pfd[1]); |
| _exit(ret); |
| } else if (pid < 0) { |
| @@ -738,27 +744,28 @@ static FILE *popen_(const struct filecache_entry *entry_p, pid_t *cpid) |
| /* This is the parent process. */ |
| close(pfd[1]); /* Close unused write end */ |
| *cpid = pid; |
| - return fdopen(pfd[0], "r"); |
| + return pfd[0]; |
| |
| error: |
| - if (fd >= 0) |
| - close(fd); |
| if (pfd[0] >= 0) |
| close(pfd[0]); |
| if (pfd[1] >= 0) |
| close(pfd[1]); |
| |
| - return NULL; |
| + return -1; |
| } |
| |
| /*! |
| ***************************************************************************** |
| * |
| ****************************************************************************/ |
| -static int pclose_(FILE *fp, pid_t pid) |
| +static int pclose_(struct io_context *op) |
| { |
| - fclose(fp); |
| - return __stop_child(pid); |
| + if (op->pfd[0] != -1) { |
| + close(op->pfd[0]); |
| + op->pfd[0] = -1; |
| + } |
| + return __stop_child(op->pid); |
| } |
| |
| /* Size of file in first volume number in which it exists */ |
| @@ -1508,10 +1515,11 @@ static int lread_rar(char *buf, size_t size, off_t offset, |
| if (sync_thread_read(op)) |
| return -EIO; |
| /* If there is still no data assume something went wrong. |
| - * Most likely CRC errors or an invalid password in the case |
| - * of encrypted aechives. |
| + * I/O buffer might simply be full and cannot receive more |
| + * data or otherwise most likely CRC errors or an invalid |
| + * password in the case of encrypted aechives. |
| */ |
| - if (op->buf->offset == offset_saved) |
| + if (op->buf->offset == offset_saved && !iob_full(op->buf)) |
| return -EIO; |
| } |
| if ((off_t)(offset + size) > op->buf->offset) { |
| @@ -1550,32 +1558,29 @@ static int lread_rar(char *buf, size_t size, off_t offset, |
| /* Take control of reader thread */ |
| if (sync_thread_noread(op)) |
| return -EIO; |
| - if (!feof(op->fp) && offset > op->buf->offset) { |
| + if (offset > op->buf->offset) { |
| /* consume buffer */ |
| op->pos += op->buf->used; |
| op->buf->ri = op->buf->wi; |
| op->buf->used = 0; |
| - (void)iob_write(op->buf, op->fp, IOB_SAVE_HIST); |
| + (void)iob_write(op->buf, op->pfd[0]); |
| sched_yield(); |
| } |
| |
| - if (!feof(op->fp)) { |
| - op->buf->ri = offset & (IOB_SZ - 1); |
| - op->buf->used -= (offset - op->pos); |
| - op->pos = offset; |
| + op->buf->ri = offset & (IOB_SZ - 1); |
| + op->buf->used -= (offset - op->pos); |
| + op->pos = offset; |
| |
| - /* Pull in rest of data if needed */ |
| - if ((size_t)(op->buf->offset - offset) < size) |
| - (void)iob_write(op->buf, op->fp, |
| - IOB_SAVE_HIST); |
| - } |
| + /* Pull in rest of data if needed */ |
| + if ((size_t)(op->buf->offset - offset) < size) |
| + (void)iob_write(op->buf, op->pfd[0]); |
| } |
| |
| if (size) { |
| int off = offset - op->pos; |
| n += iob_read(buf, op->buf, size, off); |
| op->pos += (off + size); |
| - if (__wake_thread(op, RD_READ_ASYNC)) |
| + if (__wake_thread(op, RD_ASYNC_READ)) |
| return -EIO; |
| } |
| |
| @@ -1619,7 +1624,6 @@ static int lrelease(struct fuse_file_info *fi) |
| close(FH_TOFD(fi->fh)); |
| printd(3, "(%05d) %s [0x%-16" PRIx64 "]\n", getpid(), "FREE", fi->fh); |
| free(FH_TOIO(fi->fh)); |
| - FH_ZERO(fi->fh); |
| return 0; |
| } |
| |
| @@ -2009,12 +2013,14 @@ static int CALLBACK extract_callback(UINT msg, LPARAM UserData, |
| LPARAM P1, LPARAM P2) |
| { |
| struct extract_cb_arg *cb_arg = (struct extract_cb_arg *)(UserData); |
| + struct io_context *op = cb_arg->arg; |
| + int ret; |
| |
| if (msg == UCM_PROCESSDATA) { |
| /* Handle the special case when asking for a quick "dry run" |
| * to test archive integrity. If all is well this will result |
| * in an ERAR_UNKNOWN error. */ |
| - if (!cb_arg->arg) { |
| + if (!op) { |
| if (!cb_arg->dry_run) { |
| cb_arg->dry_run = 1; |
| return 1; |
| @@ -2026,7 +2032,8 @@ static int CALLBACK extract_callback(UINT msg, LPARAM UserData, |
| * written after return from write() since the pipe is not |
| * opened using the O_NONBLOCK flag. |
| */ |
| - if (write((LPARAM)cb_arg->arg, (void *)P1, P2) == -1) { |
| + ret = write(op->pfd[1], (void *)P1, P2); |
| + if (ret == -1) { |
| /* |
| * Do not treat EPIPE as an error. It is the normal |
| * case when the process is terminted, ie. the pipe is |
| @@ -2094,10 +2101,8 @@ static int extract_rar(char *arch, const char *file, void *arg) |
| } |
| |
| extract_error: |
| - |
| if (hdl) |
| RARCloseArchive(hdl); |
| - |
| return ret; |
| } |
| |
| @@ -3196,7 +3201,7 @@ static int rar2_getattr(const char *path, struct stat *stbuf) |
| |
| struct filecache_entry *entry_p; |
| |
| - pthread_rwlock_rdlock(&file_access_lock); |
| + pthread_rwlock_wrlock(&file_access_lock); |
| entry_p = path_lookup(path, stbuf); |
| if (entry_p) { |
| if (entry_p != LOOP_FS_ENTRY) { |
| @@ -3226,7 +3231,7 @@ static int rar2_getattr(const char *path, struct stat *stbuf) |
| } |
| free(tmp); |
| |
| - pthread_rwlock_rdlock(&file_access_lock); |
| + pthread_rwlock_wrlock(&file_access_lock); |
| entry_p = path_lookup(path, stbuf); |
| if (entry_p) { |
| pthread_rwlock_unlock(&file_access_lock); |
| @@ -3274,7 +3279,7 @@ static int rar2_getattr2(const char *path, struct stat *stbuf) |
| |
| int res; |
| |
| - pthread_rwlock_rdlock(&file_access_lock); |
| + pthread_rwlock_wrlock(&file_access_lock); |
| if (path_lookup(path, stbuf)) { |
| pthread_rwlock_unlock(&file_access_lock); |
| dump_stat(stbuf); |
| @@ -3292,7 +3297,7 @@ static int rar2_getattr2(const char *path, struct stat *stbuf) |
| if (res) |
| return res; |
| |
| - pthread_rwlock_rdlock(&file_access_lock); |
| + pthread_rwlock_wrlock(&file_access_lock); |
| struct filecache_entry *entry_p = path_lookup(path, stbuf); |
| if (entry_p) { |
| pthread_rwlock_unlock(&file_access_lock); |
| @@ -3639,7 +3644,6 @@ static int rar2_releasedir2(const char *path, struct fuse_file_info *fi) |
| return -EIO; |
| free(FH_TOPATH(fi->fh)); |
| free(FH_TOIO(fi->fh)); |
| - FH_ZERO(fi->fh); |
| return 0; |
| } |
| |
| @@ -3662,7 +3666,6 @@ static int rar2_releasedir(const char *path, struct fuse_file_info *fi) |
| closedir(dp); |
| free(FH_TOPATH(fi->fh)); |
| free(FH_TOIO(fi->fh)); |
| - FH_ZERO(fi->fh); |
| return 0; |
| } |
| |
| @@ -3692,23 +3695,23 @@ static void *reader_task(void *arg) |
| } |
| goto restart; |
| } |
| - continue; |
| } |
| req = op->rd_req; |
| pthread_mutex_unlock(&op->rd_req_mutex); |
| |
| - printd(4, "Reader thread wakeup (fp:%p)\n", op->fp); |
| - if (req > RD_SYNC_NOREAD && !feof(op->fp)) |
| - (void)iob_write(op->buf, op->fp, IOB_SAVE_HIST); |
| - |
| + if (req == RD_TERM) |
| + goto out; |
| + printd(4, "Reader thread wakeup (fd:%d)\n", op->pfd[0]); |
| + if (req > RD_SYNC_NOREAD) |
| + (void)iob_write(op->buf, op->pfd[0]); |
| pthread_mutex_lock(&op->rd_req_mutex); |
| op->rd_req = RD_IDLE; |
| - pthread_cond_broadcast(&op->rd_req_cond); /* sync */ |
| + pthread_cond_signal(&op->rd_req_cond); /* sync */ |
| pthread_mutex_unlock(&op->rd_req_mutex); |
| } |
| |
| out: |
| - printd(4, "Reader thread stopped (fp:%p)\n", op->fp); |
| + printd(4, "Reader thread stopped (fd:%d)\n", op->pfd[0]); |
| return NULL; |
| } |
| |
| @@ -3914,7 +3917,7 @@ static int rar2_open(const char *path, struct fuse_file_info *fi) |
| fi->flags &= ~(O_CREAT | O_EXCL); |
| #endif |
| errno = 0; |
| - pthread_rwlock_rdlock(&file_access_lock); |
| + pthread_rwlock_wrlock(&file_access_lock); |
| entry_p = path_lookup(path, NULL); |
| |
| if (entry_p == NULL) { |
| @@ -3986,7 +3989,7 @@ static int rar2_open(const char *path, struct fuse_file_info *fi) |
| if (entry_p->flags.raw) { |
| fp = fopen(entry_p->rar_p, "r"); |
| if (fp != NULL) { |
| - io = malloc(sizeof(struct io_handle)); |
| + io = calloc(1, sizeof(struct io_handle)); |
| op = calloc(1, sizeof(struct io_context)); |
| if (!op || !io) |
| goto open_error; |
| @@ -3997,11 +4000,6 @@ static int rar2_open(const char *path, struct fuse_file_info *fi) |
| printd(3, "(%05d) %-8s%s [%-16p]\n", getpid(), "ALLOC", path, FH_TOCONTEXT(fi->fh)); |
| pthread_mutex_init(&op->raw_read_mutex, NULL); |
| op->fp = fp; |
| - op->pid = 0; |
| - op->seq = 0; |
| - op->buf = NULL; |
| - op->entry_p = NULL; |
| - op->pos = 0; |
| op->vno = -1; /* force a miss 1:st time */ |
| |
| /* |
| @@ -4012,7 +4010,7 @@ static int rar2_open(const char *path, struct fuse_file_info *fi) |
| * Since the file contents will never change this should save |
| * us from some user space calls! |
| */ |
| -#if 0 /* disable for now (issue #66) */ |
| +#if 1 /* disable for now (issue #66) */ |
| fi->keep_cache = 1; |
| #endif |
| |
| @@ -4029,114 +4027,103 @@ static int rar2_open(const char *path, struct fuse_file_info *fi) |
| goto open_error; |
| } |
| |
| - buf = malloc(P_ALIGN_(sizeof(struct iob) + IOB_SZ)); |
| + buf = iob_alloc(P_ALIGN_(sizeof(struct iob) + IOB_SZ)); |
| if (!buf) |
| goto open_error; |
| - IOB_RST(buf); |
| |
| - io = malloc(sizeof(struct io_handle)); |
| + io = calloc(1, sizeof(struct io_handle)); |
| op = calloc(1, sizeof(struct io_context)); |
| if (!op || !io) |
| goto open_error; |
| op->buf = buf; |
| - op->entry_p = NULL; |
| |
| - /* Open PIPE(s) and create child process */ |
| - fp = popen_(entry_p, &pid); |
| - if (fp != NULL) { |
| - FH_SETIO(fi->fh, io); |
| - FH_SETTYPE(fi->fh, IO_TYPE_RAR); |
| - FH_SETCONTEXT(fi->fh, op); |
| - printd(3, "(%05d) %-8s%s [%-16p]\n", getpid(), "ALLOC", |
| - path, FH_TOCONTEXT(fi->fh)); |
| - op->seq = 0; |
| - op->pos = 0; |
| - op->fp = fp; |
| - op->pid = pid; |
| - printd(4, "PIPE %p created towards child %d\n", |
| - op->fp, pid); |
| - |
| - pthread_mutex_init(&op->rd_req_mutex, NULL); |
| - pthread_cond_init(&op->rd_req_cond, NULL); |
| - op->rd_req = RD_IDLE; |
| + FH_SETIO(fi->fh, io); |
| + FH_SETTYPE(fi->fh, IO_TYPE_RAR); |
| + FH_SETCONTEXT(fi->fh, op); |
| + printd(3, "(%05d) %-8s%s [%-16p]\n", getpid(), "ALLOC", |
| + path, FH_TOCONTEXT(fi->fh)); |
| |
| - /* |
| - * The below will take precedence over keep_cache. |
| - * This flag will allow the filesystem to bypass the page cache using |
| - * the "direct_io" flag. This is not the same as O_DIRECT, it's |
| - * dictated by the filesystem not the application. |
| - * Since compressed archives might sometimes require fake data to be |
| - * returned in read requests, a cache might cause the same faulty |
| - * information to be propagated to sub-sequent reads. Setting this |
| - * flag will force _all_ reads to enter the filesystem. |
| - */ |
| + pthread_mutex_init(&op->rd_req_mutex, NULL); |
| + pthread_cond_init(&op->rd_req_cond, NULL); |
| + op->rd_req = RD_IDLE; |
| + fi->keep_cache = 0; |
| + |
| + /* |
| + * The below will take precedence over keep_cache. |
| + * This flag will allow the filesystem to bypass the page cache using |
| + * the "direct_io" flag. This is not the same as O_DIRECT, it's |
| + * dictated by the filesystem not the application. |
| + * Since compressed archives might sometimes require fake data to be |
| + * returned in read requests, a cache might cause the same faulty |
| + * information to be propagated to sub-sequent reads. Setting this |
| + * flag will force _all_ reads to enter the filesystem. |
| + */ |
| #if 0 /* disable for now */ |
| - if (entry_p->flags.direct_io) |
| - fi->direct_io = 1; |
| + if (entry_p->flags.direct_io) |
| + fi->direct_io = 1; |
| #endif |
| - |
| - /* Create reader thread */ |
| - if (pthread_create(&op->thread, &thread_attr, reader_task, (void *)op)) |
| - goto open_error; |
| - if (sync_thread_noread(op)) |
| - goto open_error; |
| - |
| - /* Promote to a write lock since we might need to |
| - * change the cache entry below. */ |
| - pthread_rwlock_unlock(&file_access_lock); |
| - pthread_rwlock_wrlock(&file_access_lock); |
| - |
| - buf->idx.data_p = MAP_FAILED; |
| - buf->idx.fd = -1; |
| - if (!preload_index(buf, path)) { |
| - entry_p->flags.save_eof = 0; |
| - entry_p->flags.direct_io = 0; |
| - fi->direct_io = 0; |
| - } else { |
| - /* Was the file removed ? */ |
| - if (get_save_eof(entry_p->rar_p) && !entry_p->flags.save_eof) { |
| - entry_p->flags.save_eof = 1; |
| - entry_p->flags.avi_tested = 0; |
| - } |
| + buf->idx.data_p = MAP_FAILED; |
| + buf->idx.fd = -1; |
| + if (!preload_index(buf, path)) { |
| + entry_p->flags.save_eof = 0; |
| + entry_p->flags.direct_io = 0; |
| + fi->direct_io = 0; |
| + } else { |
| + /* Was the file removed ? */ |
| + if (get_save_eof(entry_p->rar_p) && !entry_p->flags.save_eof) { |
| + entry_p->flags.save_eof = 1; |
| + entry_p->flags.avi_tested = 0; |
| } |
| + } |
| |
| - if (entry_p->flags.save_eof && !entry_p->flags.avi_tested) { |
| - if (check_avi_type(op)) |
| - entry_p->flags.save_eof = 0; |
| - entry_p->flags.avi_tested = 1; |
| - } |
| + if (entry_p->flags.save_eof && !entry_p->flags.avi_tested) { |
| + if (check_avi_type(op)) |
| + entry_p->flags.save_eof = 0; |
| + entry_p->flags.avi_tested = 1; |
| + } |
| |
| #ifdef DEBUG_READ |
| - char out_file[32]; |
| - sprintf(out_file, "%s.%d", "output", pid); |
| - op->dbg_fp = fopen(out_file, "w"); |
| + char out_file[32]; |
| + sprintf(out_file, "%s.%d", "output", pid); |
| + op->dbg_fp = fopen(out_file, "w"); |
| #endif |
| - /* |
| - * Make sure cache entry is filled in completely |
| - * before cloning it |
| - */ |
| - op->entry_p = filecache_clone(entry_p); |
| - if (!op->entry_p) |
| - goto open_error; |
| - goto open_end; |
| - } |
| + /* |
| + * Make sure cache entry is filled in completely |
| + * before cloning it |
| + */ |
| + op->entry_p = filecache_clone(entry_p); |
| + if (!op->entry_p) |
| + goto open_error; |
| + |
| + /* Open PIPE(s) and create child process */ |
| + if (popen_(op, &pid, op->pfd) == -1) |
| + goto open_error; |
| + op->pid = pid; |
| + printd(4, "PIPE %d/%d created towards child %u\n", |
| + op->pfd[0], op->pfd[1], pid); |
| + |
| + /* Create reader thread */ |
| + if (pthread_create(&op->thread, &thread_attr, reader_task, (void *)op)) |
| + goto open_error; |
| + if (sync_thread_read(op)) |
| + goto open_error; |
| + |
| + goto open_end; |
| } |
| |
| open_error: |
| pthread_rwlock_unlock(&file_access_lock); |
| - if (fp) { |
| - if (entry_p->flags.raw) |
| - fclose(fp); |
| - else |
| - pclose_(fp, pid); |
| - } |
| + if (fp && entry_p->flags.raw) |
| + fclose(fp); |
| free(io); |
| if (op) { |
| if (op->entry_p) |
| filecache_freeclone(op->entry_p); |
| + if (op->pid) |
| + pclose_(op); |
| free(op); |
| } |
| - free(buf); |
| + iob_free(buf); |
| |
| /* |
| * This is the best we can return here. So many different things |
| @@ -4526,30 +4513,25 @@ static int rar2_release(const char *path, struct fuse_file_info *fi) |
| FH_TOIO(fi->fh)->type == IO_TYPE_RAW) { |
| struct io_context *op = FH_TOCONTEXT(fi->fh); |
| free(FH_TOPATH(fi->fh)); |
| - if (op->fp) { |
| - if (op->entry_p->flags.raw) { |
| - printd(3, "Closing file handle %p\n", op->fp); |
| - fclose(op->fp); |
| - pthread_mutex_destroy(&op->raw_read_mutex); |
| - } else { |
| - if (!pthread_cancel(op->thread)) |
| - pthread_join(op->thread, NULL); |
| - |
| - pthread_cond_destroy(&op->rd_req_cond); |
| - pthread_mutex_destroy(&op->rd_req_mutex); |
| - |
| - if (pclose_(op->fp, op->pid)) |
| - printd(4, "child closed abnormally\n"); |
| - printd(4, "PIPE %p closed towards child %05d\n", |
| - op->fp, op->pid); |
| -#ifdef DEBUG_READ |
| - fclose(op->dbg_fp); |
| -#endif |
| - } |
| + if (op->fp && op->entry_p->flags.raw) { |
| + printd(3, "Closing file handle %p\n", op->fp); |
| + fclose(op->fp); |
| + pthread_mutex_destroy(&op->raw_read_mutex); |
| } |
| printd(3, "(%05d) %s [0x%-16" PRIx64 "]\n", getpid(), "FREE", fi->fh); |
| if (op->buf) { |
| - /* XXX clean up */ |
| + __wake_thread(op, RD_TERM); |
| + pthread_join(op->thread, NULL); |
| + pthread_cond_destroy(&op->rd_req_cond); |
| + pthread_mutex_destroy(&op->rd_req_mutex); |
| + |
| + if (pclose_(op)) |
| + printd(4, "child closed abnormally\n"); |
| + printd(4, "PIPE %d/%d closed towards child %u\n", |
| + op->pfd[0], op->pfd[1], op->pid); |
| +#ifdef DEBUG_READ |
| + fclose(op->dbg_fp); |
| +#endif |
| #ifdef HAVE_MMAP |
| if (op->buf->idx.data_p != MAP_FAILED && |
| op->buf->idx.mmap) |
| @@ -4561,12 +4543,11 @@ static int rar2_release(const char *path, struct fuse_file_info *fi) |
| free(op->buf->idx.data_p); |
| if (op->buf->idx.fd != -1) |
| close(op->buf->idx.fd); |
| - free(op->buf); |
| + iob_free(op->buf); |
| } |
| filecache_freeclone(op->entry_p); |
| free(op); |
| free(FH_TOIO(fi->fh)); |
| - FH_ZERO(fi->fh); |
| } else { |
| return lrelease(fi); |
| } |
