GNOME Bugzilla – Bug 719455
g_file_make_directory_with_parents() can erroneously throw G_IO_ERROR_EXISTS
Last modified: 2014-12-19 06:35:56 UTC
Created attachment 262986 [details] Illustration of the problem If two processes are calling g_file_make_directory_with_parents() on two different directories that have a common ancestor directory that does not exist yet, then one of the calls sometimes fails with G_IO_ERROR_EXISTS (from the race to create the common ancestor) even though the directory specified by the GFile has not been created. That's convoluted but the attached Python script should illustrate it. Basically it's a violation of the invariant of "G_IO_ERROR_EXISTS means g_file_make_directory_with_parents() was not necessary because all the necessary directories already existed."
Created attachment 262990 [details] [review] Patch that fixes the problem Here's a patch.
+ if (my_error != NULL && my_error->code == G_IO_ERROR_EXISTS) You also need to check the error domain. Use |if (g_error_matches(...))|.
Created attachment 263478 [details] [review] Updated patch
Created attachment 263479 [details] [review] Apply code review comment to surrounding code as well This patch also applies the comment about using g_error_matches() to the rest of the surrounding code in that function.
Ping?
Review of attachment 263479 [details] [review]: This is a nice clean up. Thanks.
Review of attachment 263478 [details] [review]: The handling of 'my_error' in this function is extremely confusing (particularly with respect to how it is allowed to 'leak' out of this loop, past the for loop, all the way to the end), but I believe this patch to be correct... If you're up for it, we could use another patch adding some comments about what's going on there (or you could roll it into this one if you prefer).
Sure, I'd be happy to add some clarifying comments to this patch. Thanks very much for the review.
Created attachment 292011 [details] [review] Here's a separate patch that explains the flow of my_error. --- gfile: Explain nonobvious use of my_error In g_file_make_directory_with_parents(), the my_error variable is used for several different purposes throughout the whole function, not all of which are obvious. This explains the situation with some comments.
I've committed all three patches, with some minor style changes to the comments. Thanks for the help!
Awesome, thank you!