Avoid undefined behavior due to dangling file descriptor

Authored by tobiasdeiminger on Sep 17 2018, 12:32 PM.



We request a inhibit lock of DBus type UNIX_FD from systemd logind. It's wrapped into (and owned by) a QDBusUnixFileDescriptor object of automatic storage. The file descriptor will be closed in QDBusUnixFileDescriptor Dtor, and may be reused by some other facility (e.g. pulseaudio).

If we want to store the lock longer than QDBusUnixFileDescriptor lifetime, we have to dup the file descriptor. If we don't dup, and close the original fd later in PresentationWidget::allowPowerManagement, bad things may happen.

Besides that, what we get from systemd is really a file descriptor, not a "cookie". So I renamed the m_sleepInhibitCookie to m_sleepInhibitFd and changed initial state to -1 accordingly.

BUG 393478
BUG 398720

Test Plan
  • bugs don't occur any longer
  • inhibiting sleep during presentation mode still works

Diff Detail

R223 Okular
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
Restricted Application added a project: Okular. · View Herald TranscriptSep 17 2018, 12:32 PM
Restricted Application added a subscriber: okular-devel. · View Herald Transcript
tobiasdeiminger requested review of this revision.Sep 17 2018, 12:32 PM
anthonyfieroni added inline comments.

It should be != -1

Fix check for m_sleepInhibitFd

tobiasdeiminger marked an inline comment as done.Sep 17 2018, 1:02 PM
tobiasdeiminger added inline comments.

You're right, thanks

aacid accepted this revision.Sep 18 2018, 6:00 PM

Awesome find!

I guess i should close the pulse/gstreamer bugs :D

Please commit to stable branch and merge to master

This revision is now accepted and ready to land.Sep 18 2018, 6:00 PM
This revision was automatically updated to reflect the committed changes.
tobiasdeiminger marked an inline comment as done.