Use mount point returned from DBus instead of using property value
AbandonedPublic

Authored by nicolasfella on Apr 10 2019, 2:02 PM.

Details

Summary

There is a slight time frame (~2ms) where mount has returned but the MountPoints property is not updated. When the mounpoint is queried during that time it will be empty and opening a plugged in USB device in Dolphin will fail.
To avoid this use the mountpoint information returned by the mount() call

Diff Detail

Repository
R245 Solid
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
nicolasfella created this revision.Apr 10 2019, 2:02 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 10 2019, 2:02 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
nicolasfella requested review of this revision.Apr 10 2019, 2:02 PM
This revision was not accepted when it landed; it landed in state Needs Review.Apr 10 2019, 2:07 PM
This revision was automatically updated to reflect the committed changes.
nicolasfella reopened this revision.Apr 10 2019, 2:07 PM
broulik added inline comments.Apr 10 2019, 2:19 PM
src/solid/devices/backends/udisks2/udisksstorageaccess.cpp
35

Initialization not needed for complex types

82

Won't this bypass the luks stuff when it is not mounted or something?
On mount you do this holderDevice dance but otherwise you just go straight to MountPoints

151

Coding style: if (!...) {

200

Don't you need to clear the m_mountPoint after unmount?

bruns added a comment.Apr 10 2019, 6:31 PM

This approach is completely wrong.

The right approach is to wait for the information in a PropertiesChanged signal, and only when the mountpoint has been set in the property propage the signal.

This whole "our information is inconsistent, lets query for it explicitly" dance is a mess.

This approach is completely wrong.

The right approach is to wait for the information in a PropertiesChanged signal, and only when the mountpoint has been set in the property propage the signal.

This whole "our information is inconsistent, lets query for it explicitly" dance is a mess.

They key issue is that the PropertiesChanged signal hasn't been emitted by the time filePath() is called. We could wait for the signal there, but we don't need to since we already get the same information from the mount() call, but we just didn't use it before. That won't work if the drive is already mounted by the time solid is started, so we query the mountPoints property in that case, but that is no different to the old code.

nicolasfella abandoned this revision.Sep 6 2019, 2:00 PM

The issue has been fixed in another commit