Hmm, the ndb glue change here looks decidedly wrong to me, the thing is
reference counted for a reason (guess this shows just how much attention I've
been paying to ndb changes, and I guess that needs to change from now on)
--
You are receiving this because you are subscribed to this thread.
FYI: I had to revert the ndb glue change, as it caused segfaults if just an
index dbi got closed. The commit doesn't contain any information about the
problem it tries to solve, do you remember why your change was necessary?
--
You are receiving this because you are subscribed to this thread.
Got it. I jumped to the wrong conclusion seeing sigaction() being used together
with `rpmsigTbl`. I missed that that particular line was saving the old signal
disposition, not setting the new one.
That definitely makes more sense for the name "signal queue" :-).
--
You are receiving this
Oh, rpm doesn't do clean-up from signal handlers, that's specifically
documented as being unsafe for BDB in particular. Rpm has a "signal queue"
which is polled from here and there in the codepaths and that's where cleanup
occurs. The race here is that the signal queue for trapping those
Agreed, fab7348 removes the need for my commit.
I notice exit() does not seem to be on the list of POSIX signal-safe functions
(in contrast with `_exit()`).
I guess in general, if rpm is really trying to clean up inside a signal
handler, I'd expect a lot of really hairy edge cases.
E.g.
Finally got around to investigate the rpmdb leak thing properly...
@ffesti pointed out that perhaps a better fix is to ensure the new handle is
always added to the list. And indeed, this actually fixes another problem too:
there was a window where a newly opened database would not be properly
Closed #359.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/359#event-1661156271___
Rpm-maint mailing list
Eh. It can cause a bit of awkwardness or need for discussion when you get
unlucky, agreed. So far I'm not very keen to send 3+ different PRs for small
fixes to cleanup when they don't depend on each other.
I appreciate the attention to detail. Cleanup fixes can be fiddly to get right
even
The holdup here has been the rpmdbClose() changes, I've gotten rather wary of
changes to that area due to recent fix-regression-fix-regression round - see
commit 4c6e51e2c0e3deeb052ae3c47115b6d10cb0d696 which your change almost
reverts, and then the subsequent fixes and reverts in the history.
Seems okay, do a rebase for all checks to pass, hopefully this should be
sufficient for it to be merged. @pmatilai , anything else holding this one off?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/359
-- Commit Summary --
* ndb glue: closeEnv() should always clear rdb->db_dbenv
* rpmidxHandleObsolete(): Fix fd leak in error path
* Fix leak with openDatabase() /
11 matches
Mail list logo
