From: Eric Biggers Date: Mon, 19 Oct 2015 00:39:34 +0000 (-0500) Subject: Make WIMStructs reference-counted X-Git-Tag: v1.8.3~39 X-Git-Url: https://wimlib.net/git/?p=wimlib;a=commitdiff_plain;h=3a0b45be3635a632f3acd97b170dead2de29e8a8 Make WIMStructs reference-counted Simplify the library API by eliminating rules on when WIMStructs can be freed after calls to wimlib_export_image() and wimlib_reference_resources(). Also eliminate the internal 'subwims' list. --- diff --git a/include/wimlib.h b/include/wimlib.h index ac42e81a..93e44d75 100644 --- a/include/wimlib.h +++ b/include/wimlib.h @@ -2717,8 +2717,7 @@ wimlib_delete_path(WIMStruct *wim, int image, * * Note: after calling this function, the exported WIM image(s) cannot be * independently modified because the image metadata will be shared between the - * two ::WIMStruct's. In addition, @p dest_wim will depend on @p src_wim, so @p - * src_wim cannot be freed until @p dest_wim is ready to be freed as well. + * two ::WIMStruct's. * * Note: no changes are committed to disk until wimlib_write() or * wimlib_overwrite() is called. @@ -3071,11 +3070,14 @@ wimlib_extract_xml_data(WIMStruct *wim, FILE *fp); /** * @ingroup G_general * - * Free all memory allocated for a WIMStruct and close all files associated with - * it. + * Release a reference to a ::WIMStruct. If the ::WIMStruct is still referenced + * by other ::WIMStruct's (e.g. following calls to wimlib_export_image() or + * wimlib_reference_resources()), then the library will free it later, when the + * last reference is released; otherwise it is freed immediately and any + * associated file descriptors are closed. * * @param wim - * Pointer to the ::WIMStruct to free. If @c NULL, no action is taken. + * Pointer to the ::WIMStruct to release. If @c NULL, no action is taken. */ extern void wimlib_free(WIMStruct *wim); @@ -3760,11 +3762,7 @@ wimlib_reference_resource_files(WIMStruct *wim, * @param ref_flags * Reserved; must be 0. * - * @return 0 on success; a ::wimlib_error_code value on failure. On success, - * the ::WIMStruct's of the @p resource_wims are referenced internally by @p wim - * and must not be freed with wimlib_free() or overwritten with - * wimlib_overwrite() until @p wim has been freed with wimlib_free(), or - * immediately before freeing @p wim with wimlib_free(). + * @return 0 on success; a ::wimlib_error_code value on failure. */ extern int wimlib_reference_resources(WIMStruct *wim, WIMStruct **resource_wims, diff --git a/include/wimlib/wim.h b/include/wimlib/wim.h index 4cf6779f..00de8a53 100644 --- a/include/wimlib/wim.h +++ b/include/wimlib/wim.h @@ -67,6 +67,11 @@ struct WIMStruct { * referenced from other WIMStructs. */ struct blob_table *blob_table; + /* The number of references to this WIMStruct. This is equal to the + * number of resource descriptors that reference this WIMStruct, plus 1 + * if wimlib_free() still needs to be called. */ + ssize_t refcnt; + /* * The 1-based index of the currently selected image in this WIMStruct, * or WIMLIB_NO_IMAGE if no image is currently selected. @@ -105,16 +110,6 @@ struct WIMStruct { u8 decompressor_ctype; u32 decompressor_max_block_size; - /* - * 'subwims' is the list of dependent WIMStructs (linked by - * 'subwim_node') that have been opened by calls to - * wimlib_reference_resource_files(). These WIMStructs must be retained - * so that resources from them can be used. They are internal to the - * library and are not visible to API users. - */ - struct list_head subwims; - struct list_head subwim_node; - /* Temporary field; use sparingly */ void *private; @@ -190,6 +185,9 @@ static inline bool wim_is_pipable(const WIMStruct *wim) return (wim->hdr.magic == PWM_MAGIC); } +extern void +finalize_wim_struct(WIMStruct *wim); + extern bool wim_has_solid_resources(const WIMStruct *wim); diff --git a/src/blob_table.c b/src/blob_table.c index 39193b42..1509a835 100644 --- a/src/blob_table.c +++ b/src/blob_table.c @@ -168,11 +168,18 @@ void blob_release_location(struct blob_descriptor *blob) { switch (blob->blob_location) { - case BLOB_IN_WIM: + case BLOB_IN_WIM: { + struct wim_resource_descriptor *rdesc = blob->rdesc; + list_del(&blob->rdesc_node); - if (list_empty(&blob->rdesc->blob_list)) - FREE(blob->rdesc); + if (list_empty(&rdesc->blob_list)) { + wimlib_assert(rdesc->wim->refcnt > 0); + if (--rdesc->wim->refcnt == 0) + finalize_wim_struct(rdesc->wim); + FREE(rdesc); + } break; + } case BLOB_IN_FILE_ON_DISK: #ifdef __WIN32__ case BLOB_IN_WINNT_FILE_ON_DISK: @@ -710,6 +717,8 @@ load_solid_info(WIMStruct *wim, if (ret) goto out_free_rdescs; + wim->refcnt += num_rdescs; + *rdescs_ret = rdescs; *num_rdescs_ret = num_rdescs; return 0; @@ -750,9 +759,12 @@ static void free_solid_rdescs(struct wim_resource_descriptor **rdescs, size_t num_rdescs) { if (rdescs) { - for (size_t i = 0; i < num_rdescs; i++) - if (list_empty(&rdescs[i]->blob_list)) + for (size_t i = 0; i < num_rdescs; i++) { + if (list_empty(&rdescs[i]->blob_list)) { + rdescs[i]->wim->refcnt--; FREE(rdescs[i]); + } + } FREE(rdescs); } } @@ -984,6 +996,7 @@ read_blob_table(WIMStruct *wim) goto oom; wim_reshdr_to_desc_and_blob(&reshdr, wim, rdesc, cur_blob); + wim->refcnt++; } /* cur_blob is now a blob bound to a resource. */ diff --git a/src/extract.c b/src/extract.c index 0d7ec687..532d1b1f 100644 --- a/src/extract.c +++ b/src/extract.c @@ -1929,6 +1929,7 @@ wimlib_extract_image_from_pipe_with_progress(int pipe_fd, goto out_wimlib_free; wim_reshdr_to_desc_and_blob(&reshdr, pwm, metadata_rdesc, imd->metadata_blob); + pwm->refcnt++; if (i == image) { /* Metadata resource is for the image being extracted. diff --git a/src/reference.c b/src/reference.c index ab0bab2b..a380d4ee 100644 --- a/src/reference.c +++ b/src/reference.c @@ -5,7 +5,7 @@ */ /* - * Copyright (C) 2013, 2014 Eric Biggers + * Copyright (C) 2013, 2014, 2015 Eric Biggers * * This file is free software; you can redistribute it and/or modify it under * the terms of the GNU Lesser General Public License as published by the Free @@ -37,7 +37,6 @@ struct reference_info { WIMStruct *dest_wim; struct list_head new_blobs; - struct list_head new_subwims; int ref_flags; struct blob_table *src_table; }; @@ -48,29 +47,14 @@ init_reference_info(struct reference_info *info, WIMStruct *dest_wim, { info->dest_wim = dest_wim; INIT_LIST_HEAD(&info->new_blobs); - INIT_LIST_HEAD(&info->new_subwims); info->ref_flags = ref_flags; } -static void -commit_reference_info(struct reference_info *info) -{ - list_splice(&info->new_subwims, &info->dest_wim->subwims); -} - static void rollback_reference_info(struct reference_info *info) { - WIMStruct *subwim; struct blob_descriptor *blob; - while (!list_empty(&info->new_subwims)) { - subwim = list_first_entry(&info->new_subwims, - WIMStruct, subwim_node); - list_del(&subwim->subwim_node); - wimlib_free(subwim); - } - while (!list_empty(&info->new_blobs)) { blob = list_first_entry(&info->new_blobs, struct blob_descriptor, blob_table_list); @@ -80,16 +64,6 @@ rollback_reference_info(struct reference_info *info) } } -static int -commit_or_rollback_reference_info(struct reference_info *info, int ret) -{ - if (unlikely(ret)) - rollback_reference_info(info); - else - commit_reference_info(info); - return ret; -} - static bool need_blob(const struct reference_info *info, const struct blob_descriptor *blob) { @@ -103,12 +77,6 @@ reference_blob(struct reference_info *info, struct blob_descriptor *blob) list_add(&blob->blob_table_list, &info->new_blobs); } -static void -reference_subwim(struct reference_info *info, WIMStruct *subwim) -{ - list_add(&subwim->subwim_node, &info->new_subwims); -} - static int blob_clone_if_new(struct blob_descriptor *blob, void *_info) { @@ -154,7 +122,9 @@ wimlib_reference_resources(WIMStruct *wim, WIMStruct **resource_wims, break; } - return commit_or_rollback_reference_info(&info, ret); + if (unlikely(ret)) + rollback_reference_info(&info); + return ret; } static int @@ -185,7 +155,7 @@ reference_resource_path(struct reference_info *info, const tchar *path, info->src_table = src_wim->blob_table; for_blob_in_table(src_wim->blob_table, blob_gift, info); - reference_subwim(info, src_wim); + wimlib_free(src_wim); return 0; } @@ -269,5 +239,7 @@ wimlib_reference_resource_files(WIMStruct *wim, else ret = reference_resource_paths(&info, paths_or_globs, count, open_flags); - return commit_or_rollback_reference_info(&info, ret); + if (unlikely(ret)) + rollback_reference_info(&info); + return ret; } diff --git a/src/wim.c b/src/wim.c index e77d7170..e01131dd 100644 --- a/src/wim.c +++ b/src/wim.c @@ -146,12 +146,12 @@ new_wim_struct(void) if (!wim) return NULL; + wim->refcnt = 1; filedes_invalidate(&wim->in_fd); filedes_invalidate(&wim->out_fd); wim->out_solid_compression_type = wim_default_solid_compression_type(); wim->out_solid_chunk_size = wim_default_solid_chunk_size( wim->out_solid_compression_type); - INIT_LIST_HEAD(&wim->subwims); return wim; } @@ -867,6 +867,20 @@ can_modify_wim(WIMStruct *wim) return 0; } +/* Free a WIMStruct after no more resources reference it. */ +void +finalize_wim_struct(WIMStruct *wim) +{ + if (filedes_valid(&wim->in_fd)) + filedes_close(&wim->in_fd); + if (filedes_valid(&wim->out_fd)) + filedes_close(&wim->out_fd); + wimlib_free_decompressor(wim->decompressor); + xml_free_info_struct(wim->xml_info); + FREE(wim->filename); + FREE(wim); +} + /* API function documented in wimlib.h */ WIMLIBAPI void wimlib_free(WIMStruct *wim) @@ -874,31 +888,22 @@ wimlib_free(WIMStruct *wim) if (!wim) return; - while (!list_empty(&wim->subwims)) { - WIMStruct *subwim; - - subwim = list_entry(wim->subwims.next, WIMStruct, subwim_node); - list_del(&subwim->subwim_node); - wimlib_free(subwim); - } - - if (filedes_valid(&wim->in_fd)) - filedes_close(&wim->in_fd); - if (filedes_valid(&wim->out_fd)) - filedes_close(&wim->out_fd); + /* The blob table and image metadata are freed immediately, but other + * members of the WIMStruct such as the input file descriptor are + * retained until no more exported resources reference the WIMStruct. */ free_blob_table(wim->blob_table); - - wimlib_free_decompressor(wim->decompressor); - - FREE(wim->filename); - xml_free_info_struct(wim->xml_info); - if (wim->image_metadata) { - for (unsigned i = 0; i < wim->hdr.image_count; i++) + wim->blob_table = NULL; + if (wim->image_metadata != NULL) { + for (int i = 0; i < wim->hdr.image_count; i++) put_image_metadata(wim->image_metadata[i], NULL); FREE(wim->image_metadata); + wim->image_metadata = NULL; } - FREE(wim); + + wimlib_assert(wim->refcnt > 0); + if (--wim->refcnt == 0) + finalize_wim_struct(wim); } static bool