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). You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany-plugins/pull/605
-- Commit Summary --
* projectorganizer: Close dir created with g_dir_open() in some special cases
-- File Changes --
M projectorganizer/src/prjorg-project.c (2)
-- Patch Links --
https://github.com/geany/geany-plugins/pull/605.patch https://github.com/geany/geany-plugins/pull/605.diff
@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
``` first/second/third/.../1024-th ```
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.
@techee pushed 1 commit.
db91515 projectorganizer: Don't keep directories open when enumerating their children
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.
@kugel- Good, thanks for testing.
@frlan OK to merge?
Merged #605.
github-comments@lists.geany.org