Update mount point after mount operations
ClosedPublic
Actions

Authored by davidedmundson on May 1 2018, 9:17 PM.

Details

Reviewers

broulik
mart

Group Reviewers

Plasma

Commits

R245:d735708ff11c: Update mount point after mount operations

Summary

The order of udisks evaluation has changed from:

call Mount
propertiesChanged
mount call returns

call Mount
mount call returns
propertiesChanged

The mount has finished, but the property is not yet updated.

Solid caches properties, updating them when they change. This worked
before, but due to the re-ordering client code gets "setupDone" requests
the mount point, gets an outdated version from the cache and we get
errors. Invalidating the cache causes us to round-trip to the udisks
daemon meaning we'll have the correct values.

BUG: 370975

Test Plan

Diagnosed but with dbus-monitor trace
Asked someone on the bug report to test this

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.

davidedmundson created this revision.May 1 2018, 9:17 PM

Restricted Application added a project: Frameworks. · View Herald TranscriptMay 1 2018, 9:17 PM

Restricted Application added a subscriber: Frameworks. · View Herald Transcript

davidedmundson requested review of this revision.May 1 2018, 9:17 PM

broulik accepted this revision.May 2 2018, 12:45 PM

This revision is now accepted and ready to land.May 2 2018, 12:45 PM

mart accepted this revision.May 2 2018, 12:46 PM

Closed by commit R245:d735708ff11c: Update mount point after mount operations (authored by davidedmundson). · Explain WhyMay 2 2018, 12:49 PM

This revision was automatically updated to reflect the committed changes.

bruns added a subscriber: bruns.May 2 2018, 4:44 PM

bruns added inline comments.

src/solid/devices/backends/udisks2/udisksstorageaccess.cpp
170	This replaces one race condition with another (less likely) one. There is no guarantee the property is already updated when checkAccessibility is called. The correct approach would be to defer until the propertiesChanged signal arrives and broadcast the "setup done" afterwards. A different, but more intrusive approach would be to defer any check for the "acessible" property on the caller side, waiting for the accessibilityChanged signal.

Let me clarify

At the time of the return from udisks the property on udisks is updated but the signal saying the property is updated is not yet sent, as solid caches properties we have a wrong value locally but udisks has the correct value remotely

In D12648#257365, @davidedmundson wrote:

Let me clarify

At the time of the return from udisks the property on udisks is updated but the signal saying the property is updated is not yet sent, as solid caches properties we have a wrong value locally but udisks has the correct value remotely

There is still no guarantee the method return is equivalent/isochronous with the property change. The property is changed when you receive the propertiesChanged signal, not when the method returns. The current behaviour is an implementation detail you should not rely on.

There is still no guarantee the method return is equivalent/isochronous with the property change.

Sure there is. We know udisks has completely finished mounting a filesystem, therefore we know udisks property about whether the file system is mounted will have changed.
It's equivalent to calling QLabel::setText and then after it returns knowing you can get the right result from QLabel::text

That's not relying on an implementation detail, it's that anything else would be a definite bug.

Revision Contents
Changeset List

			Path	Packages
M			src/solid/devices/backends/udisks2/udisksdevice.h (1 line)
M			src/solid/devices/backends/udisks2/udisksdevice.cpp (7 lines)
M			src/solid/devices/backends/udisks2/udisksstorageaccess.cpp (8 lines)