> On Aug. 24, 2013, 3:17 p.m., Eike Hein wrote:
> > Hm, on the face of it, this patch doesn't really make sense ... launcher
> > items don't have an associated task, so the function should already return
> > early and the extra condition should be redundant. Unless there's a race
> > condition in the library somewhere ... but then it still wouldn't crash on
> > translating a QPoint.
> >
> > Thank you for the patch, but I really still have to find a way to reproduce
> > this bug before just applying this blindly - it might be treating a symptom
> > instead of addressing the root cause.
>
> Bhushan Shah wrote:
> Are you sure that this happens on 32 bit only?
>
> Eike Hein wrote:
> No, I'm not - except I can't reproduce it on any of my 64 bit machines,
> and I have to assume if it were a widespread bug, the number of reports we'd
> be getting would be much, much higher. Note that we didn't even get any
> reports through any of the pre-releases about this, AFAIR.
>
> I'm typing this from a cellphone right now, but I hope when I get home
> tonight I'll finally get around to setting up a 32 bit VM, and I'm hoping
> it'll crash there so I can throw all the gdb/valgrind/asan at it we got :).
>
> Thomas Lübking wrote:
> What bug and is "WId" involved?
>
> Bhushan Shah wrote:
> https://bugs.kde.org/show_bug.cgi?id=322283
>
> Hrvoje Senjan wrote:
> @Thomas
> bug#322283
Thanks.
>From a brief review of libtaskmanager, TaskGroup::getMemberById(int id)
>returns AbstractGroupableItem* which could be (reimplemented)
- TaskGroup*
- LauncherItem*
- TaskItem*
but is happily static_cast'ed to TaskItem* and then accessed.
The fact(?) that itemType returns LauncherItemType indicates that there can
very well be different returns and then you're accessing random memory portions
- it does absolutely not matter that the function pointer for ::task() points
into garbage when the item is actually a Launcher - the garbage is still rather
not null, there's no guarantee that this basic deref somehow would crash or
fail (virtual ::task() needed to be moved to AbstractGroupableItem then)
To know whether or why this has different implications on different
architectures:
Valgrind will tell you.
For the moment
- either ::getMemberById() is supposed to return different types than TaskItem
here, then static_cast needs to be qobject_cast or dynamic_cast (pending on
linkage)
- or and ::getMemberById() that is *not* TaskItem must be considered a bug,
then i'd start by adding an
Q_ASSERT(!static_cast<TaskManager::AbstractGroupableItem*>(taskItem->itemType()
== TaskManager::TaskItemType))
I don't really know, but as this thing seems to manage all kinds of items, but
only updating rects for TaskItem's (and actually *every* grouped TaskItem for a
TaskGroupType return ... so we don't get more bugs on "windows don't minimize
to proper icon ;-P") makes sense, the correct solution is probably indeed:
AbstractGroupableItem *item = rootGroup()->getMemberById(id);
if (item->itemType() == TaskManager::LauncherItemType)
return; // launchers have no windows, we just cause X11 errors and with a
little luck a stupid gobject abort
if (item->itemType() == TaskManager::TaskGroupType) {
// build a WId list of all grouped Items
...
} else //if (item->itemType() == TaskManager::TaskItemType) {
tasks << static_cast<TaskManager::TaskItem*>(taskItem)->task();
}
// search parrent and other juggling to figure the proper rectangle
...
- Thomas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112241/#review38481
-----------------------------------------------------------
On Aug. 24, 2013, 2 p.m., Bhushan Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112241/
> -----------------------------------------------------------
>
> (Updated Aug. 24, 2013, 2 p.m.)
>
>
> Review request for kde-workspace, Plasma and Eike Hein.
>
>
> Description
> -------
>
> Fix the crash in plasma-desktop caused by newer QML taskbar widget.
>
> Simple steps to reproduce this crash.
>
> 1) Pin any task/application to taskbar using "show launcher when not running"
> option.
> 2) Close application.
> 3) Desktop crashes.
>
> Reason :
>
> 1) In Tasks::itemGeometryChanged(int,int,int,int,int) line 300 it checks for
> three conditions,
>
> -> pointer to task is not null
> -> taskItem itself is not null
> -> scene is not null
>
> 2) This condition gets false when item is LauncherItem. In function later
> line 334 when calling iconRect.moveTopLeft(QPoint) function it gets crashed.
>
> Patch :
>
> This patch adds check in if condition to check if taskItem is
> TaskManager::LauncherItemType and return from function if this is launcher
> item.
>
>
> Diffs
> -----
>
> plasma/desktop/applets/tasks/tasks.cpp c4aef4b
>
> Diff: http://git.reviewboard.kde.org/r/112241/diff/
>
>
> Testing
> -------
>
> Testing
>
> compilation - check
> installation - check
> plasmoidviewer - check
> in panel - check
> independently - check
>
>
> Thanks,
>
> Bhushan Shah
>
>
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
