Summary: | kwin crashed when changing the Colors Scheme. | ||
---|---|---|---|
Product: | [Plasma] kwin | Reporter: | Shlomi Fish <shlomif> |
Component: | general | Assignee: | KWin default assignee <kwin-bugs-null> |
Status: | RESOLVED FIXED | ||
Severity: | crash | CC: | 576810415, alboz, andrew.wilkin1, helderbelchior25, hrvoje.senjan, kevin.clevenger, lmpetrie, mail, mknecht, rb03884 |
Priority: | NOR | Keywords: | drkonqi |
Version: | 4.10.4 | Flags: | thomas.luebking:
ReviewRequest+
|
Target Milestone: | --- | ||
Platform: | Mageia RPMs | ||
OS: | Linux | ||
URL: | https://git.reviewboard.kde.org/r/111374/ | ||
Latest Commit: | http://commits.kde.org/kde-workspace/a2bab28a2ce61b8714349665b553cab8d5888fa7 | Version Fixed In: | 4.11 |
Description
Shlomi Fish
2013-06-24 19:39:08 UTC
It's because effects.cpp EffectsHandlerImpl::reconfigure() calls KServiceTypeTrader::query() threaded - as scritpting did (and now still does in parallel, causing this crash because KConfigGroup is not *sigh* ;-) *** Bug 321644 has been marked as a duplicate of this bug. *** *** Bug 321719 has been marked as a duplicate of this bug. *** Git commit a2bab28a2ce61b8714349665b553cab8d5888fa7 by Thomas Lübking. Committed on 02/07/2013 at 21:06. Pushed by luebking into branch 'master'. don't query effects threaded calls sycoca/kconfiggroup... FIXED-IN: 4.11 REVIEW: 111374 M +1 -0 kwin/effects.cpp http://commits.kde.org/kde-workspace/a2bab28a2ce61b8714349665b553cab8d5888fa7 *** Bug 322308 has been marked as a duplicate of this bug. *** @Thomas, fix safe for 4.10 backport? > fix safe for 4.10 backport?
There won't be another 4.10 release
Distros can still pick it up* The patch is safe (just waits until a thread finished, no races expectable) but i've no idea whether it applies. Maybe remove the empty line below the function to restore line numbers (unless there's a dozen of patches anyway) *Yes, I kow.... It would go(?): @@ -186,6 +186,7 @@ void EffectsHandlerImpl::reconfigure() QFutureWatcher<KService::List> *watcher = new QFutureWatcher<KService::List>(this); connect(watcher, SIGNAL(finished()), this, SLOT(slotEffectsQueried())); watcher->setFuture(QtConcurrent::run(KServiceTypeTrader::self(), &KServiceTypeTrader::query, QString("KWin/Effect"), QString())); + watcher->waitForFinished(); // TODO: remove once KConfigGroup is thread safe, bug #321576 } void EffectsHandlerImpl::slotEffectsQueried() @Martin, aware of it ;-) Patch *to me* seemed like a good candidate for adding to 4.10 packages, that was the intention of the question. Off course, if you two think it's a bad idea, i certainly won't add it. > Patch *to me* seemed like a good candidate for adding to 4.10 packages, that
> was the intention of the question. Off course, if you two think it's a bad
> idea, i certainly won't add it.
ah, sorry that was a misunderstanding. Yes, that is a safe and good candidate
for inclusion.
(In reply to comment #9) > It would go(?): The patch is correct and trivial enough to be safe, but if you intend to ship it to many ppl. you can do us a favor by + watcher->waitForFinished(); // TODO: remove once KConfigGroup is thread safe, bug #321576 } - void EffectsHandlerImpl::slotEffectsQueried() Ie. remove the one line to restore line numbers in backtraces (unless, as mentioned, the file differs from the vanilla anyway) *** Bug 322398 has been marked as a duplicate of this bug. *** (In reply to comment #10) > ah, sorry that was a misunderstanding. Well, i guess i could have explained my intention in the first place :-) >Yes, that is a safe and good > candidate > for inclusion. Good :-) (In reply to comment #11) > (In reply to comment #9) > > It would go(?): > > The patch is correct and trivial enough to be safe, but if you intend to > ship it to many ppl. you can do us a favor by OK, adjusted, thanks for the hint (haven't thought about that). Sorry for nagging you, and if this was inappropriate to discuss here. And off course, thanks for the input. P.S. (if it matters, distro in question is openSUSE 12.3 - for potential future reports. Though it'll take some time for it to turn up in the update channel) (In reply to comment #13) > Sorry for nagging you, and if this was inappropriate to discuss here. And > off course, thanks for the input. No, i personally consider this the way it should be. Instead of "risking" a backport, you rather asked on a public channel that's read by the maintainer and all relevant developers (and records the process) Also the bug is fixed and closed, so any "spam" (actually related discussion) is pretty harmless (there's nothing we still really need to track and no ongoing interrogatio on the issue would be disturbed) *** Bug 322535 has been marked as a duplicate of this bug. *** *** Bug 322552 has been marked as a duplicate of this bug. *** *** Bug 323556 has been marked as a duplicate of this bug. *** *** Bug 322077 has been marked as a duplicate of this bug. *** *** Bug 325673 has been marked as a duplicate of this bug. *** |