From 051a59e4c6d114fc7abfa14ff78436adab5defb9 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 12 Nov 2012 18:03:17 -0600 Subject: [PATCH] Make different threads use different FILE*'s read_wim_resource() was actually not thread safe because it expects to use the FILE * WIMStruct.fp. This commit introduces a table of FILE *'s for each WIMStruct that are only used if a special flag WIMLIB_RESOURCE_FLAG_MULTITHREADED is passed to read_wim_resource(). --- src/extract.c | 3 +- src/mount.c | 11 +++-- src/ntfs-apply.c | 4 +- src/resource.c | 110 ++++++++++++++++++++++++++++++++++-------- src/symlink.c | 4 +- src/wim.c | 20 +++++++- src/wimlib_internal.h | 16 ++++-- tests/test-imagex | 3 +- 8 files changed, 137 insertions(+), 34 deletions(-) diff --git a/src/extract.c b/src/extract.c index 7fe23ef7..6e341cc0 100644 --- a/src/extract.c +++ b/src/extract.c @@ -230,7 +230,8 @@ static int extract_symlink(const struct dentry *dentry, const char *output_path, const WIMStruct *w) { char target[4096]; - ssize_t ret = inode_readlink(dentry->d_inode, target, sizeof(target), w); + ssize_t ret = inode_readlink(dentry->d_inode, target, + sizeof(target), w, 0); if (ret <= 0) { ERROR("Could not read the symbolic link from dentry `%s'", dentry->full_path_utf8); diff --git a/src/mount.c b/src/mount.c index 27f6551a..dc1e7d91 100644 --- a/src/mount.c +++ b/src/mount.c @@ -1056,7 +1056,8 @@ static int wimfs_getxattr(const char *path, const char *name, char *value, if (res_size > size) return -ERANGE; - ret = read_full_wim_resource(lte, (u8*)value); + ret = read_full_wim_resource(lte, (u8*)value, + WIMLIB_RESOURCE_FLAG_MULTITHREADED); if (ret != 0) return -EIO; @@ -1334,6 +1335,8 @@ static int wimfs_read(const char *path, char *buf, size_t size, } else { /* Read from WIM */ + wimlib_assert(fd->f_lte->resource_location == RESOURCE_IN_WIM); + u64 res_size = wim_resource_size(fd->f_lte); if (offset > res_size) @@ -1342,7 +1345,8 @@ static int wimfs_read(const char *path, char *buf, size_t size, size = min(size, res_size - offset); if (read_wim_resource(fd->f_lte, (u8*)buf, - size, offset, false) != 0) + size, offset, + WIMLIB_RESOURCE_FLAG_MULTITHREADED) != 0) return -EIO; return size; } @@ -1396,7 +1400,8 @@ static int wimfs_readlink(const char *path, char *buf, size_t buf_len) if (!inode_is_symlink(inode)) return -EINVAL; - ret = inode_readlink(inode, buf, buf_len, ctx->wim); + ret = inode_readlink(inode, buf, buf_len, ctx->wim, + WIMLIB_RESOURCE_FLAG_MULTITHREADED); if (ret > 0) ret = 0; return ret; diff --git a/src/ntfs-apply.c b/src/ntfs-apply.c index db629d94..9b880a3d 100644 --- a/src/ntfs-apply.c +++ b/src/ntfs-apply.c @@ -72,7 +72,7 @@ extract_wim_resource_to_ntfs_attr(const struct lookup_table_entry *lte, while (bytes_remaining) { u64 to_read = min(bytes_remaining, WIM_CHUNK_SIZE); - ret = read_wim_resource(lte, buf, to_read, offset, false); + ret = read_wim_resource(lte, buf, to_read, offset, 0); if (ret != 0) break; sha1_update(&ctx, buf, to_read); @@ -310,7 +310,7 @@ static int apply_reparse_data(ntfs_inode *ni, const struct dentry *dentry, p = put_u16(p, wim_resource_size(lte)); /* ReparseDataLength */ p = put_u16(p, 0); /* Reserved */ - ret = read_full_wim_resource(lte, p); + ret = read_full_wim_resource(lte, p, 0); if (ret != 0) return ret; diff --git a/src/resource.c b/src/resource.c index 43ecc0a8..1871cfcc 100644 --- a/src/resource.c +++ b/src/resource.c @@ -421,6 +421,57 @@ u8 *put_resource_entry(u8 *p, const struct resource_entry *entry) return p; } +static FILE *wim_get_fp(WIMStruct *w) +{ + pthread_mutex_lock(&w->fp_tab_mutex); + FILE *fp; + + wimlib_assert(w->filename != NULL); + + for (size_t i = 0; i < w->num_allocated_fps; i++) { + if (w->fp_tab[i]) { + fp = w->fp_tab[i]; + w->fp_tab[i] = NULL; + goto out; + } + } + DEBUG("Opening extra file descriptor to `%s'", w->filename); + fp = fopen(w->filename, "rb"); + if (!fp) + ERROR_WITH_ERRNO("Failed to open `%s'", w->filename); +out: + pthread_mutex_unlock(&w->fp_tab_mutex); + return fp; +} + +static int wim_release_fp(WIMStruct *w, FILE *fp) +{ + int ret = 0; + FILE **fp_tab; + + pthread_mutex_lock(&w->fp_tab_mutex); + + for (size_t i = 0; i < w->num_allocated_fps; i++) { + if (w->fp_tab[i] == NULL) { + w->fp_tab[i] = fp; + goto out; + } + } + + fp_tab = REALLOC(w->fp_tab, sizeof(FILE*) * (w->num_allocated_fps + 4)); + if (!fp_tab) { + ret = WIMLIB_ERR_NOMEM; + goto out; + } + w->fp_tab = fp_tab; + memset(&w->fp_tab[w->num_allocated_fps], 0, 4 * sizeof(FILE*)); + w->fp_tab[w->num_allocated_fps] = fp; + w->num_allocated_fps += 4; +out: + pthread_mutex_unlock(&w->fp_tab_mutex); + return ret; +} + /* * Reads some data from the resource corresponding to a WIM lookup table entry. * @@ -428,13 +479,11 @@ u8 *put_resource_entry(u8 *p, const struct resource_entry *entry) * @buf: Buffer into which to write the data. * @size: Number of bytes to read. * @offset: Offset at which to start reading the resource. - * @raw: If %true, compressed data is read literally rather than being - * decompressed first. * * Returns zero on success, nonzero on failure. */ int read_wim_resource(const struct lookup_table_entry *lte, u8 buf[], - size_t size, u64 offset, bool raw) + size_t size, u64 offset, int flags) { int ctype; int ret = 0; @@ -442,7 +491,7 @@ int read_wim_resource(const struct lookup_table_entry *lte, u8 buf[], /* We shouldn't be allowing read over-runs in any part of the library. * */ - if (raw) + if (flags & WIMLIB_RESOURCE_FLAG_RAW) wimlib_assert(offset + size <= lte->resource_entry.size); else wimlib_assert(offset + size <= lte->resource_entry.original_size); @@ -453,23 +502,38 @@ int read_wim_resource(const struct lookup_table_entry *lte, u8 buf[], * the lte->wim member. The resource may be either compressed * or uncompressed. */ wimlib_assert(lte->wim != NULL); - wimlib_assert(lte->wim->fp != NULL); + + if (flags & WIMLIB_RESOURCE_FLAG_MULTITHREADED) { + fp = wim_get_fp(lte->wim); + if (!fp) + return WIMLIB_ERR_OPEN; + } else { + wimlib_assert(lte->wim->fp != NULL); + fp = lte->wim->fp; + } + ctype = wim_resource_compression_type(lte); wimlib_assert(ctype != WIM_COMPRESSION_TYPE_NONE || (lte->resource_entry.original_size == lte->resource_entry.size)); - if (raw || ctype == WIM_COMPRESSION_TYPE_NONE) - ret = read_uncompressed_resource(lte->wim->fp, + if ((flags & WIMLIB_RESOURCE_FLAG_RAW) + || ctype == WIM_COMPRESSION_TYPE_NONE) + ret = read_uncompressed_resource(fp, lte->resource_entry.offset + offset, size, buf); else - ret = read_compressed_resource(lte->wim->fp, + ret = read_compressed_resource(fp, lte->resource_entry.size, lte->resource_entry.original_size, lte->resource_entry.offset, ctype, size, offset, buf); + if (flags & WIMLIB_RESOURCE_FLAG_MULTITHREADED) { + int ret2 = wim_release_fp(lte->wim, fp); + if (ret == 0) + ret = ret2; + } break; case RESOURCE_IN_STAGING_FILE: case RESOURCE_IN_FILE_ON_DISK: @@ -534,9 +598,10 @@ int read_wim_resource(const struct lookup_table_entry *lte, u8 buf[], * * Returns 0 on success; nonzero on failure. */ -int read_full_wim_resource(const struct lookup_table_entry *lte, u8 buf[]) +int read_full_wim_resource(const struct lookup_table_entry *lte, u8 buf[], + int flags) { - return read_wim_resource(lte, buf, wim_resource_size(lte), 0, false); + return read_wim_resource(lte, buf, wim_resource_size(lte), 0, flags); } /* Chunk table that's located at the beginning of each compressed resource in @@ -748,7 +813,8 @@ finish_wim_resource_chunk_tab(struct chunk_table *chunk_tab, */ static int write_wim_resource(struct lookup_table_entry *lte, FILE *out_fp, int out_ctype, - struct resource_entry *out_res_entry) + struct resource_entry *out_res_entry, + int flags) { u64 bytes_remaining; u64 original_size; @@ -783,10 +849,14 @@ static int write_wim_resource(struct lookup_table_entry *lte, * without decompressing and recompressing the data). */ raw = (wim_resource_compression_type(lte) == out_ctype && out_ctype != WIM_COMPRESSION_TYPE_NONE); - if (raw) + + if (raw) { + flags |= WIMLIB_RESOURCE_FLAG_RAW; bytes_remaining = old_compressed_size; - else + } else { + flags &= ~WIMLIB_RESOURCE_FLAG_RAW; bytes_remaining = original_size; + } /* Empty resource; nothing needs to be done, so just return success. */ if (bytes_remaining == 0) @@ -859,7 +929,7 @@ static int write_wim_resource(struct lookup_table_entry *lte, offset = 0; do { u64 to_read = min(bytes_remaining, WIM_CHUNK_SIZE); - ret = read_wim_resource(lte, buf, to_read, offset, raw); + ret = read_wim_resource(lte, buf, to_read, offset, flags); if (ret != 0) goto out_fclose; if (!raw) @@ -928,7 +998,7 @@ static int write_wim_resource(struct lookup_table_entry *lte, goto out_fclose; } ret = write_wim_resource(lte, out_fp, WIM_COMPRESSION_TYPE_NONE, - out_res_entry); + out_res_entry, flags); if (ret != 0) goto out_fclose; if (fflush(out_fp) != 0) { @@ -994,7 +1064,7 @@ static int write_wim_resource_from_buffer(const u8 *buf, u64 buf_size, lte.attached_buffer = (u8*)buf; zero_out_hash(lte.hash); - ret = write_wim_resource(<e, out_fp, out_ctype, out_res_entry); + ret = write_wim_resource(<e, out_fp, out_ctype, out_res_entry, 0); if (ret != 0) return ret; copy_hash(hash, lte.hash); @@ -1021,7 +1091,7 @@ int extract_wim_resource_to_fd(const struct lookup_table_entry *lte, int fd, while (bytes_remaining) { u64 to_read = min(bytes_remaining, WIM_CHUNK_SIZE); - ret = read_wim_resource(lte, buf, to_read, offset, false); + ret = read_wim_resource(lte, buf, to_read, offset, 0); if (ret != 0) break; sha1_update(&ctx, buf, to_read); @@ -1075,7 +1145,7 @@ int copy_resource(struct lookup_table_entry *lte, void *wim) ret = write_wim_resource(lte, w->out_fp, wim_resource_compression_type(lte), - <e->output_resource_entry); + <e->output_resource_entry, 0); if (ret != 0) return ret; lte->out_refcnt = lte->refcnt; @@ -1108,7 +1178,7 @@ int write_dentry_resources(struct dentry *dentry, void *wim_p) lte = inode_stream_lte(dentry->d_inode, i, w->lookup_table); if (lte && ++lte->out_refcnt == 1) { ret = write_wim_resource(lte, w->out_fp, ctype, - <e->output_resource_entry); + <e->output_resource_entry, 0); if (ret != 0) break; } @@ -1179,7 +1249,7 @@ int read_metadata_resource(WIMStruct *w, struct image_metadata *imd) } /* Read the metadata resource into memory. (It may be compressed.) */ - ret = read_full_wim_resource(metadata_lte, buf); + ret = read_full_wim_resource(metadata_lte, buf, 0); if (ret != 0) goto out_free_buf; diff --git a/src/symlink.c b/src/symlink.c index 3e89a88e..b1c5b5d3 100644 --- a/src/symlink.c +++ b/src/symlink.c @@ -163,7 +163,7 @@ out: * The dentry may be either "real" symlink or a junction point. */ ssize_t inode_readlink(const struct inode *inode, char *buf, size_t buf_len, - const WIMStruct *w) + const WIMStruct *w, int read_resource_flags) { const struct lookup_table_entry *lte; int ret; @@ -178,7 +178,7 @@ ssize_t inode_readlink(const struct inode *inode, char *buf, size_t buf_len, return -EIO; u8 res_buf[wim_resource_size(lte)]; - ret = read_full_wim_resource(lte, res_buf); + ret = read_full_wim_resource(lte, res_buf, read_resource_flags); if (ret != 0) return -EIO; return get_symlink_name(res_buf, wim_resource_size(lte), buf, diff --git a/src/wim.c b/src/wim.c index d065a590..bb76f855 100644 --- a/src/wim.c +++ b/src/wim.c @@ -25,6 +25,7 @@ */ #include "config.h" +#include #include #include @@ -57,7 +58,14 @@ static int print_files(WIMStruct *w) WIMStruct *new_wim_struct() { - return CALLOC(1, sizeof(WIMStruct)); + WIMStruct *w = CALLOC(1, sizeof(WIMStruct)); + if (pthread_mutex_init(&w->fp_tab_mutex, NULL) != 0) { + ERROR_WITH_ERRNO("Failed to initialize mutex"); + FREE(w); + w = NULL; + } + return w; + } /* @@ -407,7 +415,7 @@ static int begin_read(WIMStruct *w, const char *in_wim_path, int open_flags) DEBUG("Reading the WIM file `%s'", in_wim_path); - w->filename = STRDUP(in_wim_path); + w->filename = realpath(in_wim_path, NULL); if (!w->filename) { ERROR("Failed to allocate memory for WIM filename"); return WIMLIB_ERR_NOMEM; @@ -569,6 +577,14 @@ WIMLIBAPI void wimlib_free(WIMStruct *w) if (w->out_fp) fclose(w->out_fp); + if (w->fp_tab) { + for (size_t i = 0; i < w->num_allocated_fps; i++) + if (w->fp_tab[i]) + fclose(w->fp_tab[i]); + FREE(w->fp_tab); + } + pthread_mutex_destroy(&w->fp_tab_mutex); + free_lookup_table(w->lookup_table); FREE(w->filename); diff --git a/src/wimlib_internal.h b/src/wimlib_internal.h index edadad2a..8e193216 100644 --- a/src/wimlib_internal.h +++ b/src/wimlib_internal.h @@ -30,6 +30,8 @@ #include "util.h" #include "list.h" +#include + struct stat; struct dentry; struct inode; @@ -248,12 +250,19 @@ struct image_metadata { }; +#define WIMLIB_RESOURCE_FLAG_RAW 0x1 +#define WIMLIB_RESOURCE_FLAG_MULTITHREADED 0x2 + /* The opaque structure exposed to the wimlib API. */ typedef struct WIMStruct { /* A pointer to the file indicated by @filename, opened for reading. */ FILE *fp; + FILE **fp_tab; + size_t num_allocated_fps; + pthread_mutex_t fp_tab_mutex; + /* FILE pointer for the WIM file that is being written. */ FILE *out_fp; @@ -426,9 +435,10 @@ extern u8 *put_resource_entry(u8 *p, const struct resource_entry *entry); extern int read_uncompressed_resource(FILE *fp, u64 offset, u64 size, u8 buf[]); extern int read_wim_resource(const struct lookup_table_entry *lte, u8 buf[], - size_t size, u64 offset, bool raw); + size_t size, u64 offset, int flags); -extern int read_full_wim_resource(const struct lookup_table_entry *lte, u8 buf[]); +extern int read_full_wim_resource(const struct lookup_table_entry *lte, + u8 buf[], int flags); extern int extract_wim_resource_to_fd(const struct lookup_table_entry *lte, int fd, u64 size); @@ -456,7 +466,7 @@ void free_security_data(struct wim_security_data *sd); /* symlink.c */ ssize_t inode_readlink(const struct inode *inode, char *buf, size_t buf_len, - const WIMStruct *w); + const WIMStruct *w, int read_resource_flags); extern void *make_symlink_reparse_data_buf(const char *symlink_target, size_t *len_ret); extern int inode_set_symlink(struct inode *inode, diff --git a/tests/test-imagex b/tests/test-imagex index fcbf5406..2601d876 100755 --- a/tests/test-imagex +++ b/tests/test-imagex @@ -19,7 +19,8 @@ imagex_info() { } cleanup() { - rm -rf dir* tmp* *.wim *.swm + fusermount -u tmp &> /dev/null || true + rm -rf dir* tmp* *.wim *.swm &> /dev/null || true } cleanup -- 2.43.0