Summary: | kde-base/pykde4 patch gets fork() backwards and subtly breaks DBus | ||
---|---|---|---|
Product: | Gentoo Linux | Reporter: | Hector Martin <marcan> |
Component: | [OLD] KDE | Assignee: | Gentoo KDE team <kde> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | gentoo |
Priority: | Normal | Keywords: | PATCH |
Version: | unspecified | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Package list: | Runtime testing required: | --- | |
Attachments: | kpythonpluginfactorywrapper.c-r2 |
Description
Hector Martin
2015-07-20 09:51:45 UTC
Incidentally, the code also seems to leak pipefd[0]. That should be closed after the read(). Less of a showstopper but worth fixing too. Probably also worth checking for error (cpid < 0) and doing something reasonable in that case too. I was not able to reproduce this after going through the process of rebuilding plasma-workspace with USE=python, building pykde4 with python2_7 and installing veromix which, incidentally, also involved pulling in pulseaudio... what else could I try? Created attachment 423016 [details]
kpythonpluginfactorywrapper.c-r2
I can't build pykde4 right due due to a different issue (sigh), but either way, whether the symptom can be reproduced or not (which is a rather convoluted code path involving dbus and a bunch of other moving parts), it's clear that the code is wrong, so why not just fix it?
Attached is a corrected kpythonpluginfactorywrapper.c. It fixes the bug, the fd leak, and also better handles errors. I'm not able to test it right now (though the simpler fix used in the original report did work) due to an unrelated pykde4 build failure, but as long as it doesn't cause an obvious regression, I don't see why it doesn't make sense to just use it.
Addendum: I worked around the unrelated build failure and I can confirm that the kpythonpluginfactorywrapper.c-r2 attached above builds correctly, and that I can't reproduce the original problem with it. Sorry I can not reproduce this problem. If this is still a issue please report this upstream and re-open in the case upstream patched something. Uh, what? "Sorry, I cannot reproduce the problem, so let's close the bug, even though the code is obviously broken"? How does that make *any* sense? Anyway, Portage is now in git (which it wasn't when I filed this bug), so I'll just send a pull request. Pull request: https://github.com/gentoo/gentoo/pull/1032 New steps to reproduce are listed in the PR (demonstrating the underlying PID issue, not the effects from it which depend on subtleties in other libraries). I've tested the patch on my own system. This is not an upstream issue because the bug is in a Gentoo patch. (In reply to Hector Martin from comment #8) > Pull request: https://github.com/gentoo/gentoo/pull/1032 > > New steps to reproduce are listed in the PR (demonstrating the underlying > PID issue, not the effects from it which depend on subtleties in other > libraries). > > I've tested the patch on my own system. This is not an upstream issue > because the bug is in a Gentoo patch. Thanks. https://github.com/gentoo/gentoo/commit/22aa5c8c729841148f66fb93c2f093d240d6b9bd |