> 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. > > René J.V. Bertin wrote: > 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. Bertin wrote: > With "my" fix: > > ``` > 17906 1 400c 0 33 0 2527592 6744 - Ss > 0 ?? 0:00.02 /opt/local/bin/kdeinit5 --suicide --nofork > 17907 17906 4004 0 33 0 2641356 14844 - S > 0 ?? 0:00.10 /opt/local/libexec/kde5/kf5/klauncher --fd=9 > 505 17925 1096 4006 0 33 0 2491072 5188 - S+ > 0 ttys006 0:00.01 kwrapper5 > /Applications/MacPorts/KDE4/kwrite.app/Contents/MacOS/kwrite > 505 17926 17906 400c 0 48 0 4831548 47552 - S > 0 ?? 0:01.00 > /Applications/MacPorts/KDE4/kwrite.app/Contents/MacOS/kwrite > ```
> The existing code to fallback to exec[...], possibly even faster (no dlopen). That appears to be the case indeed: `kwrapper5 /path/to/kwrite` is faster than `kwrapper5 /path/to/libkdeinit4_kwrite.dylib`. A bit surprising, because someone still has to open that dylib even when exec'ing `kwrite` ... am I missing something? - René J.V. ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126161/#review88784 ----------------------------------------------------------- On Nov. 26, 2015, 5:20 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. 26, 2015, 5:20 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/kdeinit/CMakeLists.txt f94db71 > src/kdeinit/kdeinit5_proxy.mm PRE-CREATION > src/kdeinit/kinit.cpp a18008a > src/kdeinit/kinit_mac.mm PRE-CREATION > src/klauncher/CMakeLists.txt 746edfa > src/klauncher/klauncher.h e155f72 > src/klauncher/klauncher.cpp 8b3d343 > src/klauncher/klauncher_main.cpp f69aaa5 > src/start_kdeinit/CMakeLists.txt 46d6cb3 > src/wrapper.cpp 95b7ec2 > > 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
