Does this fix 379524 and 379324? See https://community.kde.org/Infrastructure/Phabricator#Add_special_keywords

It does indeed fix both of those bugs. I have added that in the summary now, sorry about that.

I can confirm that this patch does fix the problem with installing several fonts at once systemwide here. And fontinst still exits after some time of idleness.

Btw, there's also BUG:349673 about the same problem.

The idea of KJob reference-counting the event loop is that if you quit the application while a kjob is executing, then the process will wait for the job to finish before quitting. (Use case back then, konqueror and a HTTP download).
The reason why we don't just quit after the first job is that KMainWindow itself uses QEventLoopLocker too, increasing the refcount as long as there are visible windows.

If kfontinst has no way to quit while a job is happening, this patch is ok.
But maybe it would be better to create a QEventLoopLocker in whichever GUI class kfontinst creates?

Okay, that makes sense. kfontinst doesn't actually display a UI at all, instead it just receives dbus commands from kcm - so there is no real GUI class, it just uses QCoreApplication. I could add a QEventLoopLocker to KFI::FontInst though, but, from reading the code it seems as though there's no way to actually cancel the running kjob (nor the running fontinst). That might be a cleaner approach than completely disabling the quit lock, however.

Ah so it's kind of a daemon, that should keep running until logout? I wasn't aware of that, I thought it was a KCM. Disabling as in the first patch made sense then ;)

Either version is fine with me.

Well, it's sort of a daemon, but it doesn't run until the user logs out - there's a built-in timeout of 30 seconds that will check for any connected clients and if none are found it will kill itself. The KCM communicates with the fontinst daemon via dbus :)

Either one is fine by me as well, so pick whichever one you fancy :)

Please push the first patch ;)

Wouldn't it make sense to push this to the 5.12 and 5.13 branches as well?
After all, there are quite a lot of bug reports about this...

Should be answered by some Plasma developer though, I suppose. ;-)

It would indeed, but I will wait for input before pushing it as I'm not entirely sure how it's decided what release a patch goes in to.

Generally speaking, bugfixes go into the stable branches. If the bugfix is particularly large, complex, or risky, it might be worth it to commit to master only. For this one, I think it's okay to commit it to the Plasma/5.12 branch too.

The preferred workflow here is to commit to the earliest branch and then merge; so that would mean committing to the Plasma/5.12 branch, and then merging the Plasma/5.12 into the Plasma/5.13 branch, and then merging the Plasma/5.13 branch into master.

However since you already committed it to master, that's not an option, and it will need to be cherry-picked to Plasma/5.12, and then Plasma/5.12 merged into Plasma/5.13, with no final merge to master.

But I'll let one of the more experienced plasma developers have the final say in this matter.

Okay, so should I wait for someone else to reply before committing it to the 5.12 and 5.13 branches?

you still need to merge 5.13 to master because eventually someone will, it's the trouble with cherry-picking.

I'd suggest you use -x so it adds a line about it being a cherry-pick