From 8a75d35c464b7bbf626ad9d44bbeb7cdab86e8eb Mon Sep 17 00:00:00 2001 From: Thomas Schmitt Date: Wed, 6 Oct 2010 17:33:46 +0200 Subject: [PATCH] New API call iso_image_generator_is_running(). Prevented a potential race condition between Ecma119Image disposal by burn_source and final activities of ISO generator thread. --- libisofs/ecma119.c | 40 ++++++++++++++++++++++++++++++++++++++- libisofs/ecma119.h | 2 ++ libisofs/image.c | 6 ++++++ libisofs/image.h | 6 ++++++ libisofs/libisofs.h | 44 +++++++++++++++++++++++++++++-------------- libisofs/libisofs.ver | 1 + 6 files changed, 84 insertions(+), 15 deletions(-) diff --git a/libisofs/ecma119.c b/libisofs/ecma119.c index 20668ba..75f29b1 100644 --- a/libisofs/ecma119.c +++ b/libisofs/ecma119.c @@ -59,6 +59,12 @@ void ecma119_image_free(Ecma119Image *t) if (t == NULL) return; + + if (t->refcount > 1) { + t->refcount--; + return; + } + if (t->root != NULL) ecma119_node_free(t->root); if (t->image != NULL) @@ -1315,6 +1321,12 @@ void *write_function(void *arg) if (res <= 0) goto write_error; + target->image->generator_is_running = 0; + + /* Give up reference claim made in ecma119_image_new(). + Eventually free target */ + ecma119_image_free(target); + #ifdef Libisofs_with_pthread_exiT pthread_exit(NULL); #else @@ -1340,6 +1352,12 @@ void *write_function(void *arg) /* Invalidate the transplanted checksum buffer in IsoImage */ iso_image_free_checksums(target->image, 0); + target->image->generator_is_running = 0; + + /* Give up reference claim made in ecma119_image_new(). + Eventually free target */ + ecma119_image_free(target); + #ifdef Libisofs_with_pthread_exiT pthread_exit(NULL); #else @@ -1460,6 +1478,10 @@ int ecma119_image_new(IsoImage *src, IsoWriteOpts *opts, Ecma119Image **img) if (target == NULL) { return ISO_OUT_OF_MEM; } + /* This reference will be transfered to the burn_source and released by + bs_free_data. + */ + target->refcount = 1; /* create the tree for file caching */ ret = iso_rbtree_new(iso_file_src_cmp, &(target->files)); @@ -1923,9 +1945,24 @@ int ecma119_image_new(IsoImage *src, IsoWriteOpts *opts, Ecma119Image **img) pthread_attr_init(&(target->th_attr)); pthread_attr_setdetachstate(&(target->th_attr), PTHREAD_CREATE_JOINABLE); + /* To avoid race conditions with the caller, this mark must be set + before the thread starts. So the caller can rely on a value of 0 + really meaning that the write has ended, and not that it might not have + begun yet. + In normal processing, the value will be changed to 0 at the end of + write_function(). + */ + target->image->generator_is_running = 1; + + + /* Claim that target may not get destroyed by bs_free_data() before + write_function() is done with it */ + target->refcount++; + ret = pthread_create(&(target->wthread), &(target->th_attr), write_function, (void *) target); if (ret != 0) { + target->refcount--; iso_msg_submit(target->image->id, ISO_THREAD_ERROR, 0, "Cannot create writer thread"); ret = ISO_THREAD_ERROR; @@ -1945,6 +1982,7 @@ int ecma119_image_new(IsoImage *src, IsoWriteOpts *opts, Ecma119Image **img) target_cleanup: ; if(image_checksums_mad) /* No checksums is better than mad checksums */ iso_image_free_checksums(target->image, 0); + target->image->generator_is_running = 0; ecma119_image_free(target); return ret; } @@ -1995,7 +2033,7 @@ static void bs_free_data(struct burn_source *bs) iso_ring_buffer_get_times_full(target->buffer), iso_ring_buffer_get_times_empty(target->buffer)); - /* now we can safety free target */ + /* The reference to target was inherited from ecma119_image_new() */ ecma119_image_free(target); } diff --git a/libisofs/ecma119.h b/libisofs/ecma119.h index d2e9a68..8335845 100644 --- a/libisofs/ecma119.h +++ b/libisofs/ecma119.h @@ -345,6 +345,8 @@ typedef struct Iso_Image_Writer IsoImageWriter; struct ecma119_image { + int refcount; + IsoImage *image; Ecma119Node *root; diff --git a/libisofs/image.c b/libisofs/image.c index 111cc16..e9e2a8c 100644 --- a/libisofs/image.c +++ b/libisofs/image.c @@ -89,6 +89,7 @@ int iso_image_new(const char *name, IsoImage **image) img->checksum_end_lba = 0; img->checksum_idx_count = 0; img->checksum_array = NULL; + img->generator_is_running = 0; *image = img; return ISO_SUCCESS; } @@ -607,3 +608,8 @@ int iso_image_set_checksums(IsoImage *image, char *checksum_array, return 1; } +int iso_image_generator_is_running(IsoImage *image) +{ + return image->generator_is_running; +} + diff --git a/libisofs/image.h b/libisofs/image.h index abdf328..d9994f3 100644 --- a/libisofs/image.h +++ b/libisofs/image.h @@ -166,6 +166,12 @@ struct Iso_Image uint32_t checksum_idx_count; char *checksum_array; + /** + * Whether a write run has been started by iso_image_create_burn_source() + * and has not yet been finished. + */ + int generator_is_running; + }; diff --git a/libisofs/libisofs.h b/libisofs/libisofs.h index 3dfb1df..09b786a 100644 --- a/libisofs/libisofs.h +++ b/libisofs/libisofs.h @@ -1870,11 +1870,9 @@ int iso_write_opts_set_part_offset(IsoWriteOpts *opts, * The option set to be manipulated. * @param libjte_handle * Pointer to a struct libjte_env e.g. created by libjte_new(). - * It must stay existent from the start of image writing by + * It must stay existent from the start of image generation by * iso_image_create_burn_source() until the write thread has ended. - - * >>> ts B00928 Need a way to inquire run status independent of libburn. - + * This can be inquired by iso_image_generator_is_running(). * In order to keep the libisofs API identical with and without * libjte support the parameter type is (void *). * @return @@ -1940,6 +1938,20 @@ int iso_write_opts_set_tail_blocks(IsoWriteOpts *opts, uint32_t num_blocks); int iso_write_opts_get_data_start(IsoWriteOpts *opts, uint32_t *data_start, int flag); +/** + * Update the sizes of all files added to image. + * + * This may be called just before iso_image_create_burn_source() to force + * libisofs to check the file sizes again (they're already checked when added + * to IsoImage). It is useful if you have changed some files after adding then + * to the image. + * + * @return + * 1 on success, < 0 on error + * @since 0.6.8 + */ +int iso_image_update_sizes(IsoImage *image); + /** * Create a burn_source and a thread which immediately begins to generate * the image. That burn_source can be used with libburn as a data source @@ -1966,18 +1978,22 @@ int iso_image_create_burn_source(IsoImage *image, IsoWriteOpts *opts, struct burn_source **burn_src); /** - * Update the sizes of all files added to image. - * - * This may be called just before iso_image_create_burn_source() to force - * libisofs to check the file sizes again (they're already checked when added - * to IsoImage). It is useful if you have changed some files after adding then - * to the image. - * + * Inquire whether the image generator thread is still at work. As soon as the + * reply is 0, the caller of iso_image_create_burn_source() may assume that + * the image generation has ended. + * Nevertheless there may still be readily formatted output data pending in + * the burn_source or its consumers. So the final delivery of the image has + * also to be checked at the data consumer side,e.g. by burn_drive_get_status() + * in case of libburn as consumer. + * @param image + * The image to inquire. * @return - * 1 on success, < 0 on error - * @since 0.6.8 + * 1 generating of image stream is still in progress + * 0 generating of image stream has ended meanwhile + * + * @since 0.6.38 */ -int iso_image_update_sizes(IsoImage *image); +int iso_image_generator_is_running(IsoImage *image); /** * Creates an IsoReadOpts for reading an existent image. You should set the diff --git a/libisofs/libisofs.ver b/libisofs/libisofs.ver index cd96837..c688f2e 100644 --- a/libisofs/libisofs.ver +++ b/libisofs/libisofs.ver @@ -80,6 +80,7 @@ iso_image_fs_get_publisher_id; iso_image_fs_get_system_id; iso_image_fs_get_volset_id; iso_image_fs_get_volume_id; +iso_image_generator_is_running; iso_image_get_abstract_file_id; iso_image_get_all_boot_imgs; iso_image_get_application_id;