qbittorrent/qBittorrent — pull request #2140 by sorokin (2014-11-08 15:47:26+00:00)
Title: Fix torrent removal. Closed #2132

It was reported that qbittorrent crashes on Mac OS X when user deletes
torrents from label view when label filter is active.

The callstack of the crash is the following:

1 abort + 129
2 __assert_rtn + 321
3 QTorrentHandle::total_size() const + 124
4 TorrentModelItem::data(int, int) const + 307
5 TorrentModel::data(QModelIndex const&, int) const + 255
6 QSortFilterProxyModel::data(QModelIndex const&, int) const + 109
7 QSortFilterProxyModel::data(QModelIndex const&, int) const + 109
8 QSortFilterProxyModel::data(QModelIndex const&, int) const + 109
9 QItemDelegate::rect(QStyleOptionViewItem const&, QModelIndex const&, int) const + 75
10 QItemDelegate::sizeHint(QStyleOptionViewItem const&, QModelIndex const&) const + 172
11 TransferListDelegate::sizeHint(QStyleOptionViewItem const&, QModelIndex const&) const + 14
12 QTreeView::indexRowSizeHint(QModelIndex const&) const + 887
13 QTreeViewPrivate::layout(int, bool, bool) + 462
14 QTreeView::doItemsLayout() + 356
15 QTreeViewPrivate::updateScrollBars() + 109
16 QTreeView::scrollTo(QModelIndex const&, QAbstractItemView::ScrollHint) + 124
17 TransferListWidget::currentChanged(QModelIndex const&, QModelIndex const&) + 548
18 TransferListWidget::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) + 641
19 QMetaObject::activate(QObject*, QMetaObject const*, int, void**) + 2196
20 QItemSelectionModelPrivate::_q_rowsAboutToBeRemoved(QModelIndex const&, int, int) + 3729
21 QItemSelectionModel::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) + 398
22 QMetaObject::activate(QObject*, QMetaObject const*, int, void**) + 2196
23 QAbstractItemModel::rowsAboutToBeRemoved(QModelIndex const&, int, int) + 78
24 QAbstractItemModel::beginRemoveRows(QModelIndex const&, int, int) + 106
25 QSortFilterProxyModelPrivate::remove_proxy_interval(QVector&, QVector&, int, int, QModelIndex const&, Qt::Orientation, bool) + 58
26 QSortFilterProxyModelPrivate::remove_source_items(QVector&, QVector&, QVector const&, QModelIndex const&, Qt::Orientation, bool) + 265
27 QSortFilterProxyModelPrivate::source_items_about_to_be_removed(QModelIndex const&, int, int, Qt::Orientation) + 232
28 QMetaObject::activate(QObject*, QMetaObject const*, int, void**) + 2196
29 QAbstractItemModel::rowsAboutToBeRemoved(QModelIndex const&, int, int) + 78
30 QAbstractItemModel::beginRemoveRows(QModelIndex const&, int, int) + 106
31 QSortFilterProxyModelPrivate::remove_proxy_interval(QVector&, QVector&, int, int, QModelIndex const&, Qt::Orientation, bool) + 58
32 QSortFilterProxyModelPrivate::remove_source_items(QVector&, QVector&, QVector const&, QModelIndex const&, Qt::Orientation, bool) + 265
33 QSortFilterProxyModelPrivate::source_items_about_to_be_removed(QModelIndex const&, int, int, Qt::Orientation) + 232
34 QMetaObject::activate(QObject*, QMetaObject const*, int, void**) + 2196
35 QAbstractItemModel::rowsAboutToBeRemoved(QModelIndex const&, int, int) + 78
36 QAbstractItemModel::beginRemoveRows(QModelIndex const&, int, int) + 106
37 QSortFilterProxyModelPrivate::remove_proxy_interval(QVector&, QVector&, int, int, QModelIndex const&, Qt::Orientation, bool) + 58
38 QSortFilterProxyModelPrivate::remove_source_items(QVector&, QVector&, QVector const&, QModelIndex const&, Qt::Orientation, bool) + 265
39 QSortFilterProxyModelPrivate::source_items_about_to_be_removed(QModelIndex const&, int, int, Qt::Orientation) + 232
40 QMetaObject::activate(QObject*, QMetaObject const*, int, void**) + 2196
41 QAbstractItemModel::rowsAboutToBeRemoved(QModelIndex const&, int, int) + 78
42 QAbstractItemModel::beginRemoveRows(QModelIndex const&, int, int) + 106
43 TorrentModel::removeTorrent(QString const&) + 81
44 TorrentModel::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) + 345
45 QMetaObject::activate(QObject*, QMetaObject const*, int, void**) + 2196
46 QBtSession::deletedTorrent(QString const&) + 56
47 QBtSession::deleteTorrent(QString const&, bool) + 2855
48 TransferListWidget::deleteSelectedTorrents() + 292
49 TransferListWidget::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) + 230
50 QMetaObject::activate(QObject*, QMetaObject const*, int, void**) + 2196
51 QAction::activate(QAction::ActionEvent) + 227
52 QMenuPrivate::activateCausedStack(QList > const&, QAction*, QAction::ActionEvent, bool) + 77
53 QMenuPrivate::activateAction(QAction*, QAction::ActionEvent, bool) + 470
54 QWidget::event(QEvent*) + 687
55 QMenu::event(QEvent*) + 617
56 QApplicationPrivate::notify_helper(QObject*, QEvent*) + 194
57 QApplication::notify(QObject*, QEvent*) + 2716
58 SessionApplication::notify(QObject*, QEvent*) + 21
59 QCoreApplication::notifyInternal(QObject*, QEvent*) + 118
60 QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer&, bool) + 448
61 qt_mac_handleMouseEvent(NSEvent*, QEvent::Type, Qt::MouseButton, QWidget*, bool) + 1300
62 -[NSWindow _reallySendEvent:] + 759
63 -[NSWindow sendEvent:] + 368
64 -[QCocoaPanel sendEvent:] + 113
65 -[NSApplication sendEvent:] + 2238
66 -[QNSApplication sendEvent:] + 97
67 -[NSApplication run] + 711
68 QEventDispatcherMac::processEvents(QFlags) + 1522
69 QEventLoop::processEvents(QFlags) + 77
70 QEventLoop::exec(QFlags) + 370
71 QMenu::exec(QPoint const&, QAction*) + 103
72 TransferListWidget::displayListMenu(QPoint const&) + 8741
73 TransferListWidget::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) + 622
74 QMetaObject::activate(QObject*, QMetaObject const*, int, void**) + 2196
75 QWidget::event(QEvent*) + 3082
76 QFrame::event(QEvent*) + 45
77 QAbstractScrollArea::viewportEvent(QEvent*) + 108
78 QAbstractItemView::viewportEvent(QEvent*) + 1390
79 QTreeView::viewportEvent(QEvent*) + 218
80 QAbstractScrollAreaFilter::eventFilter(QObject*, QEvent*) + 37
81 QCoreApplicationPrivate::sendThroughObjectEventFilters(QObject*, QEvent*) + 115
82 QApplicationPrivate::notify_helper(QObject*, QEvent*) + 178
83 QApplication::notify(QObject*, QEvent*) + 5742
84 SessionApplication::notify(QObject*, QEvent*) + 21
85 QCoreApplication::notifyInternal(QObject*, QEvent*) + 118
86 qt_sendSpontaneousEvent(QObject*, QEvent*) + 45
87 qt_mac_handleMouseEvent(NSEvent*, QEvent::Type, Qt::MouseButton, QWidget*, bool) + 1378
88 -[NSWindow _reallySendEvent:] + 5682
89 -[NSWindow sendEvent:] + 368
90 -[QCocoaWindow sendEvent:] + 113
91 -[NSApplication sendEvent:] + 2238
92 -[QNSApplication sendEvent:] + 97
93 -[NSApplication run] + 711
94 QEventDispatcherMac::processEvents(QFlags) + 1522
95 QEventLoop::processEvents(QFlags) + 77
96 QEventLoop::exec(QFlags) + 370
97 QCoreApplication::exec() + 199
98 main + 3415
99 start + 52

As we can see the user deleted some torrent (48). QBtSession deleted the torrent
from libtorrent::session (47) and emitted a signal (46), that torrent is deleted.
In responce to a signal (43) The TorrentModel notifies (42) its views about a change.
After a long chain of notifications (42-6) the view requested (5) a value of
total_size from TorrentModel. QTorrentHandle is already invalid as the torrent
was removed in (47). So we've got a crash in (3).

The fix is relatively straightforward do notify TorrentModel about removal not after,
but before torrent is removed from libtorrent::session. This commit does the same
thing to TorrentSpeedMonitor.

This bug reveals a major flaw in a design: currently we have a several components all
subscribed to the torrent removal signal. Signal is delivered to them in arbitrary
order, but they access each other in the handlers of this signal. E.g. TorrentModel
can access TorrentSpeedMonitor. This is doesn't lead to a crash because
TorrentSpeedMonitor returns MAX_ETA when ETA is queried for unknown torrent.