> On Nov. 25, 2015, 8:39 a.m., David Faure wrote: > > src/kdeinit/kinit.cpp, line 1621 > > <https://git.reviewboard.kde.org/r/126161/diff/1/?file=418134#file418134line1621> > > > > Yes if you have to run a separate process which will then dlopen the > > kdeinit module, the whole purpose of kdeinit is moot. You might as well > > simplify your life by getting rid of most of the Q_OS_OSX code in this > > file and simply acting as if the kdeinit module doesn't exist, on Q_OS_OSX. > > > > The existing code to fallback to executing the binary directly will do > > exactly the same as your generic proxy, possibly even faster (no dlopen). > > René J.V. Bertin wrote: > You're undoubtedly right, I considered doing this myself. It would amount > to making the `--no-fork` option the default, no? > > I don't feel comfortable/ready for that right now, without having had a > working equivalent to (the patched) kdeinit4 out there in the wild for > observation. Can we leave your suggestion for a 2nd round of housekeeping and > cleanup? > > David Faure wrote: > Not at all, --no-fork is about kdeinit's own startup, not about the way > it starts other applications. > > In general I don't see much purpose in pushing complex code if we confirm > it to serve no purpose at all. > But I looked a bit further into it, and in fact kinit's launch() does > fork+dlopen or fork+exec, depending on whether the kdeinit module was found. > > So if fork + exec is a problem on OSX, then indeed that needs to be > addressed > But if your patch does fork + exec_helper + dlopen, then that is > unnecessarily complex. > > The simple version of what I have in mind is > http://www.davidfaure.fr/2015/kinit.osx.cpp.diff i.e. never using QLibrary on > OSX. Would that work? > > René J.V. Bertin wrote: > Well, all I can say is that with that patch nothing crashes, `kdeinit5 > --kded` still launches kded5 but as a result I now no longer see something > like (in `ps` output) > > ``` > 12980 1 400c 0 33 0 2510184 6716 - Ss > 0 ?? 0:00.03 /opt/local/bin/kdeinit5 --suicide --nofork > 12981 12980 4004 0 48 0 2641864 14856 - S > 0 ?? 0:00.12 /opt/local/libexec/kde5/kf5/klauncher --fd=9 > libkdeinit5_klauncher > ``` > > but > > ``` > 13211 1 400c 0 33 0 2527592 6724 - Ss > 0 ?? 0:00.02 /opt/local/bin/kdeinit5 --suicide --nofork > 13225 1076 4006 0 31 0 2432948 576 - S+ > 0 ttys004 0:00.00 fgrep kdeinit5 > ``` > > And `kwrapper5 /path/to/kwrite` now longer launches an kwrite process > that can be killed via `killall kwrite` or equivalent. All that is probably > to be expected, but that latter observation does feel like a regression of > sorts to me. > > > BTW, I noticed that kded5 will have to be turned into an agent too, it > has no business showing up in the Dock. > > David Faure wrote: > Yes, killall only works on linux because of the proc_settitle stuff. > > I think my approach would "fix" that "regression" for killall kwrite > because it would be a real fork'ed+exec'ed process. > > You are seeing the drawbacks of the kdeinit mechanism (e.g. killall, and > probably what the `ps` entry looks like for kwrite, too) without benefiting > from its advantages (faster startup), if you have to go through a helper > process.
Erm we have a communications problem here. No, with "my" fix, applications started through kwrapper appear as individual entries in `ps` listings, with your fix only the `kwrapper5 /path/to/command` entry shows up. Also, if your fix does a "real fork + exec", how come it doesn't run afoul of the limitations imposed on that on OS X? Only because it doesn't actually load `l` (the result of QLibrary(libpath)), meaning the crash will return the day kdeinit5 itself does something off-limits? The helper from my fix is probably much less likely to develop such symptoms. And even if it does (through Qt) it would be much easier to cure (make it not use Qt at all but only POSIX APIs). Looking at this more closely I do think that my fix could borrow from yours, and skip the whole `QLibrary l(libpath)` bit (including creating `l`) because that is being done for nothing. For the rest, using a helper does seem to give a better "emulation" of what `kdeinit5` does on Linux, no? - René J.V. ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126161/#review88784 ----------------------------------------------------------- On Nov. 25, 2015, 5:19 p.m., René J.V. Bertin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126161/ > ----------------------------------------------------------- > > (Updated Nov. 25, 2015, 5:19 p.m.) > > > Review request for KDE Software on Mac OS X and KDE Frameworks. > > > Repository: kinit > > > Description > ------- > > This patch addresses several issues with the OS X adaptations: > > -1 replaces the obsolete Q_OS_MAC with Q_OS_OSX > -2 builds the relevant applications `nongui` instead of as app bundles > -3 turns klauncher into an "agent" by setting `LSUIElement` to true > programmatically > -4 ports a patch that has been in MacPorts' `port:kdelibs4` since October > 14th 2009, which prevents a kdeinit crash that is caused by calling exec > after `fork()` in an application that has used non-POSIX APIs and/or calling > those APIs in the exec'ed application. This patch (originally by MacPorts > developers Jeremy Lainé and Jeremy Lavergne) rearranges call order and uses a > proxy application to do the actual exec. > > > Diffs > ----- > > src/klauncher/klauncher.h e155f72 > src/start_kdeinit/CMakeLists.txt 46d6cb3 > src/kdeinit/kinit.cpp a18008a > src/kdeinit/CMakeLists.txt f94db71 > src/wrapper.cpp 95b7ec2 > src/klauncher/klauncher.cpp 8b3d343 > src/klauncher/klauncher_main.cpp f69aaa5 > src/klauncher/CMakeLists.txt 746edfa > src/kdeinit/kdeinit5_proxy.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/126161/diff/ > > > Testing > ------- > > On OS X 10.9.5 with Qt 5.5.1 and KF5rameworks 5.16.0 . With this patch, > starting `kded5` will launch kdeinit5 and klauncher5 as expected, but > `kdeinit5 --kded` does not yet launch `kded5`. This is probably acceptable > for typical KF5 use on OS X (kded5 can be launched as a login item or as a > LaunchAgent) but I will have another look at why the kded isn't started. > > I am not yet able to perform further testing; practice will for instance have > to show whether point 2 above needs revision (apps that need to be installed > as app bundles). > > Similarly it will have to be seen whether point 3 above has any drawbacks. > Applications running as agents do not show up in the Dock or App Switcher. > Thus, klauncher will not be able to "turn itself into" an application that > does have a full GUI presence with my current modification. I don't know if > that's supposed to be possible though. > NB: I have been building the KDE4 klauncher in a way that makes it impossible > to construct a GUI at all, so I'm not expecting issues in KF5 as long as > klauncher's role hasn't evolved too much. > > > Thanks, > > René J.V. Bertin > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
