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

Proposed by Carlos Jose Mazieri
Status: Merged
Approved by: Alan Pope 🍺🐧🐱 πŸ¦„
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
Alan Pope 🍺🐧🐱 πŸ¦„ (community) Approve
Renato Araujo Oliveira Filho (community) Approve
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.
Revision history for this message
Renato Araujo Oliveira Filho (renatofilho) wrote :

code looks good and works nice.

review: Approve
Revision history for this message
Alan Pope 🍺🐧🐱 πŸ¦„ (popey) wrote :

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

review: Approve
Revision history for this message
Jenkins Bot (ubuntu-core-apps-jenkins-bot) :
review: Approve (continuous-integration)

Preview Diff

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

Subscribers

People subscribed via source and target branches