If the scheduler has not replied by the time the batch size is used, the
default value is 0 and the mod operation results in a Floating Point
Exception.
I argue similar precautions are not needed for the call to
m_scheduler->getRemainingTime() in updateRemainingTime because the
default value of 0 will just lead to a confusing duration string but not
a crash.
To generate enough delay to witness the crash, you can simply attach a debugger to baloo_file and break on Baloo::FileIndexScheduler::getBatchSize (assuming you have debugging symbols for the baloo_file binary).
| No Linters Available |
| No Unit Test Coverage |
| Buildable 8296 | |
| Build 8314: arc lint + arc unit |
good catch, but the fix is overengineered ...
| src/qml/experimental/monitor.cpp | ||
|---|---|---|
| 90 | much more trivial: auto updateInterval = std::max(100, 5 * m_scheduler->getBatchSize());
if (m_filesIndexed % updateInterval == 0) { ... | |
| src/qml/experimental/monitor.cpp | ||
|---|---|---|
| 90 | That won't fix the issue. getBatchSize is still returning a QDBusPendingReply, using the value before it's loaded will always be zero. | |
| src/qml/experimental/monitor.cpp | ||
|---|---|---|
| 90 | It does fix the issue because if getBatchSize returns 0 then updateInterval is set to 100. It does introduce another issue which is that it won't allow for an update interval under 100 (say, if the batch size was configurable in the future which is something you should really consider) but that is a minor issue compared to the crash this fixes. | |
| src/qml/experimental/monitor.cpp | ||
|---|---|---|
| 90 | The batchsize 40 by default, so currently the update interval is 200 files. 100 or 200 does not matter to much, it is just a crude limiter anyway. A better fix would be emit the remaining time directly from the scheduler, which could adapt much better to fast and slow progress. And listening to signals on DBus is in general preferable to querying. | |
| src/qml/experimental/monitor.cpp | ||
|---|---|---|
| 90 | The batch size is hardcoded at 40 so querying it at at any interval is pointless, for that matter. Instead of hardcoding more magic numbers I would just #define BATCH_SIZE 40 inside the Engine lib and set the update interval to 5 * BATCH_SIZE. | |
| src/qml/experimental/monitor.cpp | ||
|---|---|---|
| 90 | Actually, both values are completely unrelated. They exist for similar reasons (commiting each file / updating the time for each file hurts performance). Just hardcoding e.g. 100 here would do. | |