Taking over maintainership of Baloo

[Dominik Haumann]

Hi,

indeed I also did take a look at the bug list, and out of the 200
bugs, without much exaggeration almost all of them are crashes.

In fact, I believe it's important to CC the plasma-devel mailing list
here: Given we are about to release a LTS Plasma 5.8, and baloo is
tightly integrated into Plasma, Plasma will still contain lots and
lots of crashes.

Maybe the Plasma people are also interested in taking e.g. 2-3 days to
look into this.

Greetings,
Dominik

PS: Many crashes are probably due to missing error checks, e.g.: in
postingdb.cpp:

    int rc = mdb_get(m_txn, m_dbi, &key, &val);
    if (rc == MDB_NOTFOUND) {
        return 0;
    }
    Q_ASSERT_X(rc == 0, "PostingDB::iter", mdb_strerror(rc));

    return new DBPostingIterator(val.mv_data, val.mv_size);

Looking into the API documentation, mdb_get() [1] says:
  Returns: A non-zero error value on failure and 0 on success. Some
possible errors are:
     MDB_NOTFOUND - the key was not in the database.
     EINVAL - an invalid parameter was specified.

Note the "Some possible errors", there are many many more. The code
should probably be if (rc != 0) return 0; etc. Baloo has tons of this
bugs.

[1] http://104.237.133.194/doc/group__mdb.html#ga8bf10cd91d3f3a83a34d04ce6b07992d


On Sun, Sep 11, 2016 at 7:29 PM, Christoph Cullmann <cullmann at absint.com> wrote:
> Hi,
>
>> Hi,
>>
>> On 11 September 2016 at 22:44, Christoph Cullmann <cullmann at absint.com> wrote:
>>> Hi,
>>>
>>> it would be very nice to have some active maintainership there,
>>> given a lot of VERY major crash bugs pile up there since > 1 year.
>>
>> The issue is that neither Pinak nor I can reproduce much of the bugs,
>> making solving them difficult. I'll try harder to nowadays, though ;-)
> I think the main issue is: There is no need to reproduce them, it is clear, that baloo will crash
> on any problem with lmdb, as "no" errors are handled (or lets say 99% of the errors are not handled).
>
> The code is full of Q_ASSERT, but no handling of the return codes beside that.
>
> Actually, I think, in most cases the code shall just catch the error cases and do nothing (or purge data
> until all is fine again) but not like now: crash or assert.
>
> That is not acceptable for something that runs per default ;=)
>
> Just looking at the code, you see things like:
>
> 1) Baloo::Database: needs a mutex, as many threads might call ::open (e.g. in krunner) => corruption, dead
>
> 2) inproper cleanup: I doubt one can  mdb_txn_abort(txn); if already mdb_txn_begin(...) failed
>
> 3) needs everywhere return code checks, e.g. in PostingList PostingDB::get(const QByteArray& term), we have
> tons of bugs about random crashs afterwards: https://bugs.kde.org/show_bug.cgi?id=367480
>
> 4) 32-bit systems supported at all? ATM, after 1GB of indexing, that was it, no more baloo or any other application
> calling any of the accessors of e.g. Query (if you have bad luck).
>
>>
>>> Beside that, could we get some baloo-bugs at kde.org list as default assignee
>>> for all baloo bugs instead of "one" person?
>>
>> If we're going to do it this way, why not just make it the default
>> frameworks mailing list, or the frameworks bugs list if there is one?
> I am not sure, if that will make people happy ;=)
>
> Greetings
> Christoph
>
> --
> ----------------------------- Dr.-Ing. Christoph Cullmann ---------
> AbsInt Angewandte Informatik GmbH      Email: cullmann at AbsInt.com
> Science Park 1                         Tel:   +49-681-38360-22
> 66123 Saarbrücken                      Fax:   +49-681-38360-20
> GERMANY                                WWW:   http://www.AbsInt.com
> --------------------------------------------------------------------
> Geschäftsführung: Dr.-Ing. Christian Ferdinand
> Eingetragen im Handelsregister des Amtsgerichts Saarbrücken, HRB 11234