> On July 27, 2014, 11:32 a.m., Thomas Lübking wrote: > > kdeui/util/kcrash.cpp, line 316 > > <https://git.reviewboard.kde.org/r/119497/diff/1/?file=293441#file293441line316> > > > > is libdispatch OSX only or is it also used on FreeBSD? > > > > The question (more to Michael ;-) is whether unconditionally closing > > all FDs is acceptable in general (there's hell lot of Q_OS_* items, incl. > > Q_OS_HURD ;-) or the check should be inverted to > > > > #ifdef Q_OS_LINUX > > Michael Pyne wrote: > Unfortunately closing all FDs is probably not acceptable *in general*, > but in the context of a launching a crash handler in a crashed KDE GUI > application, I don't see any reason why it would hurt, it's not as if the > application itself is going to need the FD anymore as it's about to crash and > has already run its emergency save function (and if the app does need it, > they can either use their own crash function or use the documented KeepFD > flag). Closing all FDs is a security and rlimit precaution in case Dr. Konqi > gets started directly from the crashing proc instead of via kdeinit. > > The check should probably remain on any POSIX-alike, in my opinion, as we > wouldn't want Dr. Konqi on Mac OS X accidentally having access to a crashing > application FDs if launched directly either. > > Ian Wadham wrote: > On Apple OS X it is Apple's COCOA library internals that have the problem > FDs, not the app, and COCOA crashes internally if you close its FDs in the > crash handler. We do not know what numbers those FDs have in any arbitrary > app or run of an app, and I think there is intrinsically no way to know. We > also do not know what FDs may have been created by internals of the KDE > library, but they do not seem to mind being closed peremptorily. > > I could put a setFlags call (conditional on Q_OS_MAC) a few lines earlier > in the default crash-handler, but there does not seem to be much point in > that. > > It is quite safe to leave FDs open, because KCrash closes them all > unconditionally on line 623 of kcrash.cpp (if it starts Dr K by forking), > with the comment "We are in the child now. Close FDs unconditionally." > Presumably, by that time, the COCOA library has had a chance to terminate its > internals gracefully, because this time there is no secondary crash. > Presumably also, if Dr K is started via kinit and klauncher, its FDs are all > its own, but kinit is a very grey area for me. > > Michael Pyne wrote: > Yes, you're right. It seems in the only problematic case (a direct > fork/exec to start drkonqi) that we already close FDs unconditionally. In > that case I think don't closing the FDs from the crash handler adds any value > (and I suppose, might even interfere in debugging once drkonqi can get a > debugger attached), so it might be best to remove the feature from the > default crash handler entirely. David, thoughts?
NOT closing FDs at this point has been made conditional on Q_OS_WIN or Q_OS_MAC. Others may make it unconditional on all platforms. It might be a safe thing to do... > On July 27, 2014, 11:32 a.m., Thomas Lübking wrote: > > kinit/kinit.cpp, line 1478 > > <https://git.reviewboard.kde.org/r/119497/diff/1/?file=293442#file293442line1478> > > > > this and line 1504 look like debug leftovers? or are they needed for > > extra logging? > > Ian Wadham wrote: > I was using them for diagnosing this portability bug in kinit, but I > thought it would be best to leave them in as a safety measure, so that > someone can see what happened in the event that my issue/question re line 119 > (what to do about the usage of "DISPLAY" in Apple OS X) causes premature > termination of kdeinit4 in the future in an Apple OS X environment. This part of the patch has been discontinued. - Ian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119497/#review63257 ----------------------------------------------------------- On Sept. 15, 2014, 1:39 a.m., Ian Wadham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119497/ > ----------------------------------------------------------- > > (Updated Sept. 15, 2014, 1:39 a.m.) > > > Review request for KDE Software on Mac OS X, KDE Runtime, kdelibs, and > Michael Pyne. > > > Repository: kdelibs > > > Description > ------- > > When a KDE app crashes in Apple OS X, it just disappears from the screen. At > the most, the user is invited to report the crash to Apple. AFAIK this has > been a problem in KDE on Apple OS X for years, leading to frustration with > KDE among Apple users and MacPorts developers and an attitude among KDE > developers of "Why does nobody report the problem(s) on bugs.kde.org?" > > It is my strong belief that the failure to report crashes of KDE apps in > Apple OS X also exists in Frameworks. > > So far I have identified a number of portability bugs in KDE on Apple OS X: 1 > in KCrash, 1 in kdeinit4 and 5 in Dr Konqi. Patches for the first two are > submitted in this review. Patches for three more are submitted in part 2 of > this review, against kde-runtime. I am still investigating the other two > problems in Dr Konqi - and there could be more than two... > > In this review we have two portability problems: > > 1. KCrash itself crashes (SIGILL) when it tries to close all file descriptors > and so Dr Konqi is not started. Some of the FDs belong to the Apple OS X > library (COCOA) and it crashes if they are closed prematurely. > > 2. The preferred way to start Dr K is via a socket message to kdeinit4, but > that fails in Apple OS X because kdeinit4 is listening with the wrong socket > name. The DISPLAY ID is missing from the end of the name. The fallback is for > KCrash to use fork() and exec(), which works, but could cause Dr K to be > polluted, depending on the nature of the crash. > > This "deafness" of kdeinit4 (and klauncher) could be causing other failures > in KDE software in the Apple OS X environment. > > > Diffs > ----- > > kdeui/util/kcrash.cpp 45eb46b > > Diff: https://git.reviewboard.kde.org/r/119497/diff/ > > > Testing > ------- > > Using Apple OS X 10.7.5 (Lion) on a MacBook Pro, I have installed KDE libs > via MacPorts (at version 4.12.5) and I have adapted kdesrc-build to run in an > Apple OS X environment and used it to test against the KDE 4.13 branch. I > have been testing with a KDE app that I can crash at will and using stderr > and Apple OS X Console log output to determine the outcome. > > Please note that I am the -only- KDE developer who has this kind of setup, > but I am NOT a KDE core developer. My experience before now has been in KDE > Games. However I used to be a UNIX and database guru before I retired in 1998. > > I NEED HELP from KDE -core- developers to proceed further. These problems > also exist in FRAMEWORKS, but I am as yet unable to build or test Frameworks > on Apple OS X. And I am sure there are many more Apple OS X portability > problems in KDE software. > > Without my patches, Dr Konqi is not started and, if it does get past its own > crash, KCrash reports: > KCrash: Attempting to start > /kdedev/kde4.13/kde4/lib/kde4/libexec/drkonqi.app/Contents/MacOS/drkonqi from > kdeinit > sock_file=/kdedev/kde4.13/home/.kde4.13/socket-Tara.local/kdeinit4__tmp_launch-svPd0L_org.x_0 > Warning: connect() failed: : No such file or directory > > With my patches, Dr Konqi can always be started directly (using fork()) and > it -will- start via kdeinit4 and klauncher but it immediately runs into a > privilege problem with Apple OS X (a problem which I am still investigating). > > I would not have got this far without help from Michael Pyne, Thomas Lübking > and several of the MacPorts developers, as well as the unfailing enthusiasm > and encouragement of my friend Marko Käning. > > > Thanks, > > Ian Wadham > >
