Bug 435143 - Baloo crashes often with ASSERT(!url.endsWith('/'))
Summary: Baloo crashes often with ASSERT(!url.endsWith('/'))
Status: RESOLVED FIXED
Alias: None
Product: frameworks-baloo
Classification: Frameworks and Libraries
Component: Baloo File Daemon (show other bugs)
Version: 5.80.0
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Stefan Brüns
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-03-30 11:16 UTC by Oded Arbel
Modified: 2021-04-24 21:06 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.82


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Oded Arbel 2021-03-30 11:16:59 UTC
SUMMARY

When indexing, the file indexer stops every few hours and needs to be restarted. Reviewing the logs, the problem appears to be this:

----8<----
Mar 29 23:26:33 vesho baloo_file[2071207]: ASSERT: "!url.endsWith('/')" in file [...]src/file/filewatch.cpp, line 100
----8<----

STEPS TO REPRODUCE
1. Stop the file indexer
2. Clear the baloo database
3. Start the file indexer

OBSERVED RESULT
After a while the indexer stops. Running
$ journalctl --user -u $(systemctl --user list-units | grep app-kcm_fileindexermonitor | awk '{print$1}') | grep ASSERT
shows the assertion from the log.

EXPECTED RESULT
Baloo should not crash

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: 
KDE Plasma Version: 5.21.80
KDE Frameworks Version: 5.81
Qt Version: 5.15.2

ADDITIONAL INFORMATION

The problematic code seems to be this:
----8<----
    // Directories must always end with a trailing slash '/'
    QString url = urlString;
    if (isDir) {
        Q_ASSERT(!url.endsWith('/'));
        url.append(QLatin1Char('/'));
    }
----8<----

which seems weird to me - wouldn't it be better to no crash the program in case the URL ends with '/', especially if you're going to tack it on anyway? Why not just say: if its a directory, add a slash if is missing, and just continue?
Comment 1 Oded Arbel 2021-03-30 15:33:14 UTC
While I already had the source code open, maybe this simple change will be acceptable: https://invent.kde.org/frameworks/baloo/-/merge_requests/47
Comment 2 Nate Graham 2021-04-21 19:49:30 UTC
Git commit 96284739b4b63110113af0ba7dedfd18c4459063 by Nate Graham, on behalf of Oded Arbel.
Committed on 21/04/2021 at 17:46.
Pushed by ngraham into branch 'master'.

Do not crash when a dir is move/delete is detected by kinotify

It appears that sometimes with kinotify sends a directory move or delete event,
it may be sent with a trailing slash on the path - and this causes filewatch
to fire an assertion.

Instead we might expect that things like that happen and just avoid tacking
on the required trailing slash ourselves.

M  +2    -4    src/file/filewatch.cpp

https://invent.kde.org/frameworks/baloo/commit/96284739b4b63110113af0ba7dedfd18c4459063
Comment 3 tagwerk19 2021-04-23 07:02:54 UTC
(In reply to Nate Graham from comment #2)
> https://invent.kde.org/frameworks/baloo/commit/
> 96284739b4b63110113af0ba7dedfd18c4459063
If the idea is:

   // Directories must always end with a trailing slash '/'

then I think you are probably not meaning this:

   if (isDir && url.endsWith(QLatin1Char('/'))) {
        url.append(QLatin1Char('/'));
    }
Comment 4 Oded Arbel 2021-04-24 10:54:36 UTC
(In reply to tagwerk19 from comment #3)
> then I think you are probably not meaning this:
> 
>    if (isDir && url.endsWith(QLatin1Char('/'))) {
>         url.append(QLatin1Char('/'));
>     }

You are correct. This has also been pointed out by other people. I'm working on fixing this.
Comment 5 Oded Arbel 2021-04-24 21:06:33 UTC
The last issue was fixed in https://invent.kde.org/frameworks/baloo/-/commit/dc04879f058dd1eab72e4bd4c80bdc1ea3c06c9b

@tagwerk19, thanks for noticing.