Bug#741564: libkio5: libkio : segmentation fault caused by KFileItemDelegate
On 03/14/2014 01:32 PM, Maximiliano Curia wrote:
Control: tag -1 + upstream
Control: forwarded -1 https://bugs.kde.org/show_bug.cgi?id=332132
¡Hola Paul!
El 2014-03-14 a las 12:29 +0100, Paul Chavent escribió:
I've only checked by code review, and yes the suspicious code seems unchanged in the current git tree.
Ok.
I've filled a report on the kde bug tracker : https://bugs.kde.org/show_bug.cgi?id=332132
Great, I've added the forward information to the Debian bug.
I can write a patch that workaround the problem, but I'm not sure to find
the solution the authors of this code would imagine.
Well, preparing a patch, testing it in your environment, and
submitting it to git.reviewboard.kde.org is a consistent way to get the
developers feedback.
A patch have been submitted to https://git.reviewboard.kde.org/r/116808/ and on the kde bug tracker.
I attach the patch on this thread too.
This fix the bug but it let appear an other one. If i hover the mouse on the file list, everything is fine. But as soon i get out of the file list, the application abort with a message (QWidget::repaint: Recursive repaint detected). I remember to read some peoples who already reported such a bug. I will go on investigations.
In any case, the "backtrace" that you provide could be improved adding the
corresponding states to the involved variables, so even someone foreing to
this code (like myself), can understand somthing like why is that state is not
forward and not valid.
I won't be able to use the monitor features of gdb since the bug disappear
if i attach to the process.
However i can add printf traces and still reproduce the bug. Is it what you
mean ?
Well, printf won't work, you'll need to use kDebug or kWarning. And configure
your ~/.kde/share/config/kdebugrc to get the debug output to stderr, I think
it's something like this:
[kio (delegateanimationhandler)]
InfoOutput=2
The warning should appear in the ~/.xsession-errors file.
Happy hacking,
--- a/kio/kio/delegateanimationhandler.cpp
+++ b/kio/kio/delegateanimationhandler.cpp
@@ -198,7 +198,6 @@
while (i.hasNext())
{
i.next();
- qDeleteAll(*i.value());
delete i.value();
}
animationLists.clear();
@@ -268,7 +267,7 @@
// }
}
-AnimationState *DelegateAnimationHandler::animationState(const QStyleOption &option,
+DelegateAnimationHandler::AnimationStatePtr DelegateAnimationHandler::animationState(const QStyleOption &option,
const QModelIndex &index,
const QAbstractItemView *view)
{
@@ -276,15 +275,15 @@
// item will be drawn in two locations at the same time and hovered in one and
// not the other. We can't tell them apart because they both have the same index.
if (!view || static_cast<const ProtectedAccessor*>(view)->draggingState())
- return NULL;
+ return AnimationStatePtr(NULL);
- AnimationState *state = findAnimationState(view, index);
+ AnimationStatePtr state = findAnimationState(view, index);
bool hover = option.state & QStyle::State_MouseOver;
// If the cursor has entered an item
- if (!state && hover)
+ if (state.isNull() && hover)
{
- state = new AnimationState(index);
+ state = AnimationStatePtr(new AnimationState(index));
addAnimationState(state, view);
if (!fadeInAddTime.isValid() ||
@@ -303,7 +302,7 @@
eventuallyStartIteration(index);
}
- else if (state)
+ else if (!state.isNull())
{
// If the cursor has exited an item
if (!hover && (!state->animating || state->direction == QTimeLine::Forward))
@@ -336,9 +335,9 @@
eventuallyStartIteration(index);
}
}
- else if (!state && index.model()->data(index, KDirModel::HasJobRole).toBool())
+ else if (state.isNull() && index.model()->data(index, KDirModel::HasJobRole).toBool())
{
- state = new AnimationState(index);
+ state = AnimationStatePtr(new AnimationState(index));
addAnimationState(state, view);
startAnimation(state);
state->setJobAnimation(true);
@@ -348,7 +347,7 @@
}
-AnimationState *DelegateAnimationHandler::findAnimationState(const QAbstractItemView *view,
+DelegateAnimationHandler::AnimationStatePtr DelegateAnimationHandler::findAnimationState(const QAbstractItemView *view,
const QModelIndex &index) const
{
// Try to find a list of animation states for the view
@@ -356,16 +355,16 @@
if (list)
{
- foreach (AnimationState *state, *list)
+ foreach (AnimationStatePtr state, *list)
if (state->index == index)
return state;
}
- return NULL;
+ return AnimationStatePtr(NULL);
}
-void DelegateAnimationHandler::addAnimationState(AnimationState *state, const QAbstractItemView *view)
+void DelegateAnimationHandler::addAnimationState(AnimationStatePtr state, const QAbstractItemView *view)
{
AnimationList *list = animationLists.value(view);
@@ -381,12 +380,12 @@
list->append(state);
}
-void DelegateAnimationHandler::restartAnimation(AnimationState *state)
+void DelegateAnimationHandler::restartAnimation(AnimationStatePtr state)
{
startAnimation(state);
}
-void DelegateAnimationHandler::startAnimation(AnimationState *state)
+void DelegateAnimationHandler::startAnimation(AnimationStatePtr state)
{
state->time.start();
state->animating = true;
@@ -400,10 +399,10 @@
int activeAnimations = 0;
QRegion region;
- QMutableLinkedListIterator<AnimationState*> i(*list);
+ QMutableLinkedListIterator<AnimationStatePtr> i(*list);
while (i.hasNext())
{
- AnimationState *state = i.next();
+ AnimationStatePtr state = i.next();
if (!state->animating)
continue;
@@ -427,7 +426,7 @@
// a "hover in" for the index.
if (state->direction == QTimeLine::Backward || !state->index.isValid())
{
- delete state;
+ state.clear();
i.remove();
}
}
@@ -443,7 +442,6 @@
void DelegateAnimationHandler::viewDeleted(QObject *view)
{
AnimationList *list = animationLists.take(static_cast<QAbstractItemView*>(view));
- qDeleteAll(*list);
delete list;
}
--- a/kio/kio/delegateanimationhandler_p.h
+++ b/kio/kio/delegateanimationhandler_p.h
@@ -107,7 +107,8 @@
{
Q_OBJECT
- typedef QLinkedList<AnimationState*> AnimationList;
+ typedef QSharedPointer<AnimationState> AnimationStatePtr;
+ typedef QLinkedList< AnimationStatePtr > AnimationList;
typedef QMapIterator<const QAbstractItemView *, AnimationList*> AnimationListsIterator;
typedef QMutableMapIterator<const QAbstractItemView *, AnimationList*> MutableAnimationListsIterator;
@@ -115,9 +116,9 @@
DelegateAnimationHandler(QObject *parent = 0);
~DelegateAnimationHandler();
- AnimationState *animationState(const QStyleOption &option, const QModelIndex &index, const QAbstractItemView *view);
+ AnimationStatePtr animationState(const QStyleOption &option, const QModelIndex &index, const QAbstractItemView *view);
- void restartAnimation(AnimationState* state);
+ void restartAnimation(AnimationStatePtr state);
void gotNewIcon(const QModelIndex& index);
@@ -127,9 +128,9 @@
private:
void eventuallyStartIteration(QModelIndex index);
- AnimationState *findAnimationState(const QAbstractItemView *view, const QModelIndex &index) const;
- void addAnimationState(AnimationState *state, const QAbstractItemView *view);
- void startAnimation(AnimationState *state);
+ AnimationStatePtr findAnimationState(const QAbstractItemView *view, const QModelIndex &index) const;
+ void addAnimationState(AnimationStatePtr state, const QAbstractItemView *view);
+ void startAnimation(AnimationStatePtr state);
int runAnimations(AnimationList *list, const QAbstractItemView *view);
void timerEvent(QTimerEvent *event);
void setSequenceIndex(int arg1);
--- a/kio/kio/kfileitemdelegate.cpp
+++ b/kio/kio/kfileitemdelegate.cpp
@@ -106,9 +106,9 @@
QTextLayout *labelLayout, QTextLayout *infoLayout, QRect *textBoundingRect) const;
void drawTextItems(QPainter *painter, const QTextLayout &labelLayout, const QTextLayout &infoLayout,
const QRect &textBoundingRect) const;
- KIO::AnimationState *animationState(const QStyleOptionViewItemV4 &option, const QModelIndex &index,
+ QSharedPointer<KIO::AnimationState> animationState(const QStyleOptionViewItemV4 &option, const QModelIndex &index,
const QAbstractItemView *view) const;
- void restartAnimation(KIO::AnimationState* state);
+ void restartAnimation(QSharedPointer<KIO::AnimationState> state);
QPixmap applyHoverEffect(const QPixmap &icon) const;
QPixmap transition(const QPixmap &from, const QPixmap &to, qreal amount) const;
void initStyleOption(QStyleOptionViewItemV4 *option, const QModelIndex &index) const;
@@ -584,23 +584,23 @@
animationHandler->gotNewIcon(index);
}
-void KFileItemDelegate::Private::restartAnimation(KIO::AnimationState* state)
+void KFileItemDelegate::Private::restartAnimation(QSharedPointer<KIO::AnimationState> state)
{
animationHandler->restartAnimation(state);
}
-KIO::AnimationState *KFileItemDelegate::Private::animationState(const QStyleOptionViewItemV4 &option,
+QSharedPointer<KIO::AnimationState> KFileItemDelegate::Private::animationState(const QStyleOptionViewItemV4 &option,
const QModelIndex &index,
const QAbstractItemView *view) const
{
if (!(KGlobalSettings::graphicEffectsLevel() & KGlobalSettings::SimpleAnimationEffects)) {
- return NULL;
+ return QSharedPointer<KIO::AnimationState>(NULL);
}
if (index.column() == KDirModel::Name)
return animationHandler->animationState(option, index, view);
- return NULL;
+ return QSharedPointer<KIO::AnimationState>(NULL);
}
@@ -1268,7 +1268,7 @@
// Check if the item is being animated
// ========================================================================
- KIO::AnimationState *state = d->animationState(opt, index, view);
+ QSharedPointer<KIO::AnimationState> state = d->animationState(opt, index, view);
KIO::CachedRendering *cache = 0;
qreal progress = ((opt.state & QStyle::State_MouseOver) &&
index.column() == KDirModel::Name) ? 1.0 : 0.0;
@@ -1277,7 +1277,7 @@
QIcon::State iconState = option.state & QStyle::State_Open ? QIcon::On : QIcon::Off;
QPixmap icon = opt.icon.pixmap(opt.decorationSize, iconMode, iconState);
- if (state && !state->hasJobAnimation())
+ if (!state.isNull() && !state->hasJobAnimation())
{
cache = state->cachedRendering();
progress = state->hoverProgress();
Reply to: