Re: Patch for 78542 - change thumbnails to use a thread

[Damon Chaplin <damon ximian com>]

On Tue, 2002-05-21 at 17:18, Alex Larsson wrote:


> Race condition on thumbnails_to_make:
> 
> +	info = g_new0 (NautilusThumbnailInfo, 1);
> +	info->image_uri = file_uri;
> +	info->mime_type = nautilus_file_get_mime_type (file);
> +	info->original_file_mtime = file_mtime;
> +		
> +	/* Check if it is already in the list of thumbnails to make. */
> +	if (g_list_find_custom (thumbnails_to_make, info,
> +				compare_thumbnail_info) == NULL) {
> +#ifdef DEBUG_THUMBNAILS
> +		g_message ("(Main Thread) Locking mutex\n");
> +#endif
> +		pthread_mutex_lock (&thumbnails_mutex);
>  
> You need to grab the lock before searching the list. (Or you could walk of 
> the list if you accidentally read it while the thread was modifying it.)

OK. I've fixed that.


> If convert gets stuck it blocks the thumbnailing thread. Do we care about 
> that? I think the old code had a timeout check.

I've changed the code to add a timeout before running convert which
will kill it if it doesn't exit in a reasonable time (currently 10
seconds, though I think maybe 1 or 2 minutes is better.)
I've tested this with a dummy 'convert' script that just sleeps, and it
works fine.

 
> thumbnail_thread_notify_file_changed() doesn't really need to do 
> GDK_THREADS_ENTER/LEAVE, since none of nautilus uses gdk threads. But it's 
> good practice, so lets leave it there.
> 
> I think thumbnail_thread_is_running and thumbnails_to_make need to be volatile.

OK. Fixed.


I've updated the patch on the bug.

  http://bugzilla.gnome.org/show_bug.cgi?id=78542

Damon