Merge lp:~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051 into lp:ubuntu-filemanager-app

Proposed by Carlos Jose Mazieri on 2017-01-27
Status: Merged
Approved by: Alan Pope 🍺🐧🐱 πŸ¦„ on 2017-01-27
Approved revision: 585
Merged at revision: 584
Proposed branch: lp:~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051
Merge into: lp:ubuntu-filemanager-app
Diff against target: 94 lines (+21/-10)
3 files modified
src/plugin/folderlistmodel/networklistworker.cpp (+17/-6)
src/plugin/folderlistmodel/networklistworker.h (+3/-3)
src/plugin/folderlistmodel/networklocation.cpp (+1/-1)
To merge this branch: bzr merge lp:~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051
Reviewer Review Type Date Requested Status
Jenkins Bot continuous-integration Approve on 2017-01-27
Alan Pope 🍺🐧🐱 πŸ¦„ 2017-01-27 Approve on 2017-01-27
Renato Araujo Oliveira Filho (community) Approve on 2017-01-27
Review via email: mp+315772@code.launchpad.net

Commit message

FIX the netwotk crash when Localtion::m_info is deleted in the main thread

Description of the change

FIX the netwotk crash when Localtion::m_info is deleted in the main thread because it is used in the worker thread, NetworkListWorker creates its own DirItemInfo instance and copies the data from Localtion::m_info to avoid that problem.

Location and descendant classes should be checked further to avoid deleting Localtion::m_info

To post a comment you must log in.

code looks good and works nice.

review: Approve

Tested on up to date M10, that fixed the crasher. Many thanks.

review: Approve
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/plugin/folderlistmodel/networklistworker.cpp'
2--- src/plugin/folderlistmodel/networklistworker.cpp 2015-12-12 14:32:18 +0000
3+++ src/plugin/folderlistmodel/networklistworker.cpp 2017-01-27 13:13:23 +0000
4@@ -24,15 +24,22 @@
5 #include "locationurl.h"
6
7 NetworkListWorker::NetworkListWorker(LocationItemDirIterator * dirIterator,
8- DirItemInfo * mainItemInfo, const DirItemInfo *parent) :
9+ DirItemInfo * mainItemInfo,
10+ const DirItemInfo * parentItemInfo) :
11 DirListWorker(dirIterator->path(),
12 dirIterator->filters(),
13 dirIterator->flags() == QDirIterator::Subdirectories ? true : false),
14 m_dirIterator(dirIterator),
15- m_mainItemInfo(mainItemInfo),
16- m_parent(parent)
17+ m_mainItemInfo(mainItemInfo), // m_mainItemInfo takes ownership of mainItemInfo
18+ m_parentItemInfo(0)
19 {
20 mLoaderType = NetworkLoader;
21+ // create its own instance by doing a copy from parentItemInfo
22+ if (parentItemInfo != 0)
23+ {
24+ m_parentItemInfo = new DirItemInfo();
25+ *m_parentItemInfo = *parentItemInfo;
26+ }
27 }
28
29
30@@ -40,6 +47,10 @@
31 {
32 delete m_dirIterator;
33 delete m_mainItemInfo;
34+ if (m_parentItemInfo != 0)
35+ {
36+ delete m_parentItemInfo;
37+ }
38 }
39
40
41@@ -47,7 +58,7 @@
42 {
43 DirItemInfoList netContent;
44 m_dirIterator->load();
45- bool is_parent_of_smb_url = m_parent != 0 && m_parent->urlPath().startsWith(LocationUrl::SmbURL);
46+ bool is_parent_of_smb_url = m_parentItemInfo != 0 && m_parentItemInfo->urlPath().startsWith(LocationUrl::SmbURL);
47 while (m_dirIterator->hasNext())
48 {
49 m_mainItemInfo->setFile(m_dirIterator->next());
50@@ -68,7 +79,7 @@
51 */
52 void NetworkListWorker::setSmbItemAttributes()
53 {
54- if (m_parent->isHost()) { m_mainItemInfo->setAsShare(); }
55+ if (m_parentItemInfo->isHost()) { m_mainItemInfo->setAsShare(); }
56 else
57- if (m_parent->isWorkGroup()) { m_mainItemInfo->setAsHost(); }
58+ if (m_parentItemInfo->isWorkGroup()) { m_mainItemInfo->setAsHost(); }
59 }
60
61=== modified file 'src/plugin/folderlistmodel/networklistworker.h'
62--- src/plugin/folderlistmodel/networklistworker.h 2015-12-12 14:32:18 +0000
63+++ src/plugin/folderlistmodel/networklistworker.h 2017-01-27 13:13:23 +0000
64@@ -40,15 +40,15 @@
65 public:
66 NetworkListWorker(LocationItemDirIterator * dirIterator,
67 DirItemInfo * mainItemInfo,
68- const DirItemInfo * parent = 0);
69+ const DirItemInfo * parentItemInfo = 0);
70 ~NetworkListWorker();
71 protected:
72 virtual DirItemInfoList getNetworkContent();
73 void setSmbItemAttributes();
74 protected:
75 LocationItemDirIterator * m_dirIterator;
76- DirItemInfo * m_mainItemInfo;
77- const DirItemInfo * m_parent;
78+ DirItemInfo * m_mainItemInfo; //takes ownership from mainItemInfo
79+ DirItemInfo * m_parentItemInfo; //create its own instance by doing a copy from parentItemInfo
80 };
81
82 #endif // NETWORKLISTWORKER_H
83
84=== modified file 'src/plugin/folderlistmodel/networklocation.cpp'
85--- src/plugin/folderlistmodel/networklocation.cpp 2015-12-12 14:32:18 +0000
86+++ src/plugin/folderlistmodel/networklocation.cpp 2017-01-27 13:13:23 +0000
87@@ -36,6 +36,6 @@
88
89 LocationItemDirIterator *dirIterator = newDirIterator(urlPath,filter,flags,LocationItemDirIterator::LoadLater);
90 DirItemInfo *baseitemInfo = newItemInfo(QLatin1String(0));
91-
92+ // the NetworkListWorker object takes ownership of baseitemInfo and also creates its own copy of m_info
93 return new NetworkListWorker(dirIterator, baseitemInfo, m_info);
94 }

Subscribers

People subscribed via source and target branches