This can never be reached unless something went really wrong: All paths in this method lead to exit, i.e. it will never return.

If exit somehow spawns an event loop again (exit handlers?), that's broken and will cause many "fun" issues due to accidental reentrancy.

The reasoning is kinda stupid imho…

The reasoning is:

The fcntl can only fail if the file descriptor is invalid. That means QProcess passed a bad file descriptor, so the bug would be in QProcess.

i.e. if this actually happens, it's a bug in QProcess and you can report it.

So we might as well take precautions to prevent such cases.

IMO the root cause needs to be found and it's not in QProcessPrivate::waitForDeadChild.

Anyway, I can't reproduce the "crash with 0" bug anymore, but I'm still not confident since I still get double sessionFinished calls despite exits…

You can try to trace why each of those calls happen.

So I just realized the exit we've been calling, is not the exit(2) syscall, but QCoreApplication::exit which does in fact returns, and I was able to reproduce the "crash with 0" bug. I concluded this had nothing to do with QProcess.

Sorry for this nothingburger, here is what I understood what happened:

• When UserSession QProcessed::finished, we catch it in HelperApp::sessionFinished
• If it's a QProcess::NormalExit, call QCoreApplication::exit:
  • This only signals the event loop that we requested an exit, and the return code we exit with
  • It actually returns in order to process more events
• This means we accidentally fall through our second case of QProcess::CrashExit, believing we stumbled into one:
  • By calling this exit, this sets the event loop return code to 3
• Our HelperApp then process all the remaining events
• Finally, the event loop returns to the main thread
• When main() reaches out of scope, it calls our destructor ~HelperApp()
• Which in turns calls UserSession::stop().
• However, the UserSession is already QProcess::NotRunning by this stage, so it emits another QProcess::finished(Auth::HELPER_SESSION_ERROR, QProcess::NormalExit).
  • HelperApp::sessionFinshed gets called again…

sequenceDiagram
    participant main as ::main
    participant HelperApp as SDDM::HelperApp
    participant Backend as SDDM::Backend
    participant UserSession as SDDM::UserSession

    main->>HelperApp: exec()
    Note over HelperApp: setUp() and doAuth()
    HelperApp->>Backend: openSession()
    Backend->>UserSession: start()

    Note over UserSession: Session goes on…
    UserSession->>HelperApp: QProcess::finished()
    Note over HelperApp: sessionFinished()
    opt QProcess::NormalExit
        alt exitCode not 0
            HelperApp->>HelperApp: exit(Auth::HELPER_SESSION_ERROR)
        else
            HelperApp->>HelperApp: exit(Auth::HELPER_SUCCES)
        end
    end
    Note right of HelperApp: Those RETURN and fall through (not exit syscall)
    HelperApp->>HelperApp: exit(Auth::HELPER_SESSION_ERROR)
    HelperApp-->HelperApp: Finish processing events
    HelperApp-->>main: Event loop returns with 3
    main->>main: Return 3
    Note over main: At SCOPE FINISH
    main->>HelperApp: ~HelperApp() (call destructor)
    HelperApp->>UserSession: stop()
    Note over UserSession: state() is QProcess::NotRunning
    UserSession->>HelperApp: QProcess::finished(3, 0)
    Note over HelperApp: Repeat sessionFinished() a second time

Should be pretty simple to fix:

• We should either explicitly return in HelperApp::sessionFinished, or wrap the second case in an else branch
• In the destructor, call m_session->stop() only if m_session->state() != QProcess::NotRunning.

So I just realized the exit we've been calling, is not the exit(2) syscall, but QCoreApplication::exit which does in fact return

In that case it should probably be made a bit more explicit by calling QCoreApplication::exit() instead...

However, the UserSession is already QProcess::NotRunning by this stage, so it emits another QProcess::finished(Auth::HELPER_SESSION_ERROR, QProcess::NormalExit).

That always seemed odd to me, maybe we can just remove it?

In that case it should probably be made a bit more explicit by calling QCoreApplication::exit() instead...

Done

That always seemed odd to me, maybe we can just remove it?

Replaced by a qInfo() log