projectorganizer: Close dir created with g_dir_open() in some special cases #605
Conversation
… cases At the moment the directory isn't closed when tm_get_real_path() returns NULL or when the given path was already visited (happens e.g. with symlink loops).
|
@kugel- Would you test this patch? I hope it could fix #604 - otherwise I don't know. Do you use some symlinks that could cause that the same path is re-visited many times? Or something that causes that tm_get_real_path() returns NULL for many files? These are the only two cases in which the directory wasn't properly closed (for normal projects I haven't been able to reproduce the issue - no dirs were open). Note that if you had 1024 nested directories like the current code will still need to open all the intermediate directories for traversal and you'll get the issue during project load. This would be rather hard to fix (or more precisely, the traversal would have to be slow because the intermediate dirs would have to be re-opened closed all the time) but this is a rather crazy case so I'll leave it as it is. |
…children At the moment the code calls g_dir_open() for the current dir, then recurses to its children and finally closes the dir. This makes the intermediate dirs open until the recursion returns and could potentially cause too many open fd if the directory structure is extremely deep. Instead, get all children of a directory first, close the directory and then recurse into the remembered subdirectories.
|
Disregard my previous comment about the deeply nested directories - it's easy to fix and I've added a patch that does that so now there's at most 1 directory open during traversal. Anyway, the second patch is just a bonus - it's not something that could cause that many fd's are open after the project is loaded and not the cause of the original problem. |
|
LGTM. First commit makes a lot of sense and fixes and actual possible issue. Second seems a little more theoretical and slightly slower in the regular case, but fair enough anyway. |
|
LGTM @b4n, I would have said the second would be faster, reading everything while its all cached, but anyway it hardly matters in a function doing so much IO. |
|
And to have all opinions covered, I think the real-world speed will be identical :-). |
|
Your changes seem to work. Thank you. I didn't test the first fix individually, just the HEAD of your branch. |
At the moment the directory isn't closed when tm_get_real_path() returns
NULL or when the given path was already visited (happens e.g. with
symlink loops).