Make WIMStructs reference-counted
authorEric Biggers <ebiggers3@gmail.com>
Mon, 19 Oct 2015 00:39:34 +0000 (19:39 -0500)
committerEric Biggers <ebiggers3@gmail.com>
Mon, 19 Oct 2015 01:59:18 +0000 (20:59 -0500)
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.

include/wimlib.h
include/wimlib/wim.h
src/blob_table.c
src/extract.c
src/reference.c
src/wim.c

index ac42e81..93e44d7 100644 (file)
@@ -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,
index 4cf6779..00de8a5 100644 (file)
@@ -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);
 
index 39193b4..1509a83 100644 (file)
@@ -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.  */
index 0d7ec68..532d1b1 100644 (file)
@@ -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.
index ab0bab2..a380d4e 100644 (file)
@@ -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;
 }
index e77d717..e01131d 100644 (file)
--- 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