Default to closing file descriptors when excuting commands #1097

New issue

Closed

opened 2025-05-15 12:46:00 +00:00 by jefferyto · 2 comments

jefferyto commented

2025-05-15 12:46:00 +00:00

Contributor

What I'd like to do and why

I backup from lvm snapshots and use hooks to manage these snapshots. I recently upgraded from Ubuntu 24.10 to 25.04, and so from borgmatic 1.8.9 to 1.9.14. Now I see warning messages like these in my logs:

May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 4 (pipe:[3894695]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh
May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 5 (pipe:[3958664]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh
May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 6 (pipe:[3892837]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh
May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 7 (pipe:[3961695]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh
May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 8 (pipe:[3962865]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh
May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 9 (pipe:[3967006]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh
May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 10 (pipe:[4749780]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh
May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 11 (pipe:[4755248]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh
May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 12 (pipe:[4752769]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh
May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 13 (pipe:[4757415]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh
May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 14 (pipe:[4758597]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh
May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 15 (pipe:[4760711]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh

I know work was done in #1068 and #1074 to address a similar issue for lvm commands, but rather than adding close_fds=True when running hooks, I think a better approach would be to have closed file descriptors be the default and explicitly keep them open only when necessary (i.e. when passing credentials to borg). Would this be acceptable?

Other notes / implementation ideas

No response

### What I'd like to do and why I backup from lvm snapshots and use hooks to manage these snapshots. I recently upgraded from Ubuntu 24.10 to 25.04, and so from borgmatic 1.8.9 to 1.9.14. Now I see warning messages like these in my logs: ``` May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 4 (pipe:[3894695]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 5 (pipe:[3958664]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 6 (pipe:[3892837]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 7 (pipe:[3961695]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 8 (pipe:[3962865]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 9 (pipe:[3967006]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 10 (pipe:[4749780]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 11 (pipe:[4755248]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 12 (pipe:[4752769]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 13 (pipe:[4757415]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 14 (pipe:[4758597]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh May 15 13:22:35 computer borgmatic[1390545]: WARNING File descriptor 15 (pipe:[4760711]) leaked on lvremove invocation. Parent PID 1749455: /bin/sh ``` I know work was done in #1068 and #1074 to address a similar issue for lvm commands, but rather than adding `close_fds=True` when running hooks, I think a better approach would be to have closed file descriptors be the default and explicitly keep them open only when necessary (i.e. when passing credentials to borg). Would this be acceptable? ### Other notes / implementation ideas _No response_

witten commented

2025-05-15 17:08:13 +00:00

Owner

You are correct that #1068 / #1074 should fix this issue, which means upgrading to borgmatic 2.0.6+ should address it for you. The reason though that close_fds=True was added when running hooks instead of reversing that logic and closing unneeded file descriptors by default is simply because the former was easier / less invasive to implement. A quote from the PR:

Not exactly what we talked about, but it is easier to close fds when we know that we don't need than auditing everything else since the calls often span over multiple functions and it is not easy to know when which credential is relevant (or other named pipes).

Having said that, reversing the logic would be fine with me. It'd just likely be a pain to implement.

You are correct that #1068 / #1074 should fix this issue, which means upgrading to borgmatic 2.0.6+ should address it for you. The reason though that `close_fds=True` was added when running hooks instead of reversing that logic and closing unneeded file descriptors by default is simply because the former was easier / less invasive to implement. A quote from the PR: > Not exactly what we talked about, but it is easier to close fds when we know that we don't need than auditing everything else since the calls often span over multiple functions and it is not easy to know when which credential is relevant (or other named pipes). Having said that, reversing the logic would be fine with me. It'd just likely be a pain to implement.

witten commented

2025-10-15 22:26:54 +00:00

Owner

Closing this since it sounds like it's already implemented and so an upgrade should solve it for you. If you find out that's not the case and/or you'd like to discuss further, please let me know!

witten

2025-10-15 22:26:54 +00:00