Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051 into lp:ubuntu-filemanager-app
Review: Approve Tested on up to date M10, that fixed the crasher. Many thanks. -- https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051/+merge/315772 Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051 into lp:ubuntu-filemanager-app
The proposal to merge lp:~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051 into lp:ubuntu-filemanager-app has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051/+merge/315772 -- Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051 into lp:ubuntu-filemanager-app
The proposal to merge lp:~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051 into lp:ubuntu-filemanager-app has been updated. Status: Approved => Merged For more details, see: https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051/+merge/315772 -- Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051 into lp:ubuntu-filemanager-app
Review: Approve continuous-integration -- https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051/+merge/315772 Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051 into lp:ubuntu-filemanager-app
Review: Approve code looks good and works nice. -- https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051/+merge/315772 Your team Ubuntu File Manager Developers is requested to review the proposed merge of lp:~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051 into lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~renatofilho/ubuntu-filemanager-app/fix-network-crash into lp:ubuntu-filemanager-app
Review: Disapprove Closing this MR Replaced by: https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051/+merge/315772 -- https://code.launchpad.net/~renatofilho/ubuntu-filemanager-app/fix-network-crash/+merge/314870 Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051 into lp:ubuntu-filemanager-app
Carlos Jose Mazieri has proposed merging
lp:~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051 into
lp:ubuntu-filemanager-app.
Commit message:
FIX the netwotk crash when Localtion::m_info is deleted in the main thread
Requested reviews:
Ubuntu File Manager Developers (ubuntu-filemanager-dev)
For more details, see:
https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051/+merge/315772
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
--
Your team Ubuntu File Manager Developers is requested to review the proposed
merge of lp:~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051
into lp:ubuntu-filemanager-app.
=== modified file 'src/plugin/folderlistmodel/networklistworker.cpp'
--- src/plugin/folderlistmodel/networklistworker.cpp 2015-12-12 14:32:18 +
+++ src/plugin/folderlistmodel/networklistworker.cpp 2017-01-27 13:13:23 +
@@ -24,15 +24,22 @@
#include "locationurl.h"
NetworkListWorker::NetworkListWorker(LocationItemDirIterator * dirIterator,
- DirItemInfo * mainItemInfo, const DirItemInfo *parent) :
+ DirItemInfo * mainItemInfo,
+ const DirItemInfo * parentItemInfo) :
DirListWorker(dirIterator->path(),
dirIterator->filters(),
dirIterator->flags() == QDirIterator::Subdirectories ? true : false),
m_dirIterator(dirIterator),
-m_mainItemInfo(mainItemInfo),
-m_parent(parent)
+m_mainItemInfo(mainItemInfo), // m_mainItemInfo takes ownership of mainItemInfo
+m_parentItemInfo(0)
{
mLoaderType = NetworkLoader;
+ // create its own instance by doing a copy from parentItemInfo
+ if (parentItemInfo != 0)
+ {
+ m_parentItemInfo = new DirItemInfo();
+ *m_parentItemInfo = *parentItemInfo;
+ }
}
@@ -40,6 +47,10 @@
{
delete m_dirIterator;
delete m_mainItemInfo;
+if (m_parentItemInfo != 0)
+{
+delete m_parentItemInfo;
+}
}
@@ -47,7 +58,7 @@
{
DirItemInfoList netContent;
m_dirIterator->load();
- bool is_parent_of_smb_url = m_parent != 0 && m_parent->urlPath().startsWith(LocationUrl::SmbURL);
+ bool is_parent_of_smb_url = m_parentItemInfo != 0 && m_parentItemInfo->urlPath().startsWith(LocationUrl::SmbURL);
while (m_dirIterator->hasNext())
{
m_mainItemInfo->setFile(m_dirIterator->next());
@@ -68,7 +79,7 @@
*/
void NetworkListWorker::setSmbItemAttributes()
{
-if (m_parent->isHost()) { m_mainItemInfo->setAsShare(); }
+if (m_parentItemInfo->isHost()) { m_mainItemInfo->setAsShare(); }
else
-if (m_parent->isWorkGroup()) { m_mainItemInfo->setAsHost(); }
+if (m_parentItemInfo->isWorkGroup()) { m_mainItemInfo->setAsHost(); }
}
=== modified file 'src/plugin/folderlistmodel/networklistworker.h'
--- src/plugin/folderlistmodel/networklistworker.h 2015-12-12 14:32:18 +
+++ src/plugin/folderlistmodel/networklistworker.h 2017-01-27 13:13:23 +
@@ -40,15 +40,15 @@
public:
NetworkListWorker(LocationItemDirIterator * dirIterator,
DirItemInfo * mainItemInfo,
- const DirItemInfo * parent = 0);
+ const DirItemInfo * parentItemInfo = 0);
~NetworkListWorker();
protected:
virtual DirItemInfoList getNetworkContent();
voidsetSmbItemAttributes();
protected:
LocationItemDirIterator * m_dirIterator;
- DirItemInfo * m_mainItemInfo;
- const DirItemInfo * m_parent;
+ DirItemInfo * m_mainItemInfo; //takes ownership from mainItemInfo
+ DirItemInfo * m_parentItemInfo; //create its own instance by doing a copy from parentItemInfo
};
#endif // NETWORKLISTWORKER_H
=== modified file 'src/plugin/folderlistmodel/networklocation.cpp'
--- src/plugin/folderlistmodel/networklocation.cpp 2015-12-12 14:32:18 +
+++ src/plugin/folderlistmodel/networklocation.cpp 2017-01-27 13:13:23 +
@@ -36,6 +36,6 @@
LocationItemDirIterator *dirIterator = newDirIterator(urlPath,filter,flags,LocationItemDirIterator::LoadLater);
DirItemInfo *baseitemInfo = newItemInfo(QLatin1String(0));
-
+// the NetworkListWorker object takes ownership of baseitemInfo and also creates its own copy of m_info
return new NetworkListWorker(dirIterator, baseitemInfo, m_info);
}
--
Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers
Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net
Unsubscribe :
[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051 into lp:ubuntu-filemanager-app
Carlos Jose Mazieri has proposed merging
lp:~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051 into
lp:ubuntu-filemanager-app.
Commit message:
It fixes a network crash bug #1609051 when Localtion::m_info is deleted in the
main thread.
Requested reviews:
Ubuntu File Manager Developers (ubuntu-filemanager-dev)
For more details, see:
https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051/+merge/315768
It fixes a network crash bug #1609051 when Localtion::m_info is deleted in the
main thread.
It happens because Localtion::m_info is used in the worker thread as 'parent'
item information.
Solution: 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, methods such as Location::refreshInfo() and
Location::becomeParent(), Location::setInfoItem(),
TrashLocation::becomeParent() can be changed to copy data from a temporary
DirItemInfo object.
--
Your team Ubuntu File Manager Developers is requested to review the proposed
merge of lp:~carlos-mazieri/ubuntu-filemanager-app/fix-network-crash-1609051
into lp:ubuntu-filemanager-app.
=== modified file 'src/plugin/folderlistmodel/networklistworker.cpp'
--- src/plugin/folderlistmodel/networklistworker.cpp 2015-12-12 14:32:18 +
+++ src/plugin/folderlistmodel/networklistworker.cpp 2017-01-27 12:57:59 +
@@ -24,15 +24,22 @@
#include "locationurl.h"
NetworkListWorker::NetworkListWorker(LocationItemDirIterator * dirIterator,
- DirItemInfo * mainItemInfo, const DirItemInfo *parent) :
+ DirItemInfo * mainItemInfo,
+ const DirItemInfo * parentItemInfo) :
DirListWorker(dirIterator->path(),
dirIterator->filters(),
dirIterator->flags() == QDirIterator::Subdirectories ? true : false),
m_dirIterator(dirIterator),
-m_mainItemInfo(mainItemInfo),
-m_parent(parent)
+m_mainItemInfo(mainItemInfo), // m_mainItemInfo takes ownership of mainItemInfo
+m_parentItemInfo(0)
{
mLoaderType = NetworkLoader;
+ // create its own instance by doing a copy from parentItemInfo
+ if (parentItemInfo != 0)
+ {
+ m_parentItemInfo = new DirItemInfo();
+ *m_parentItemInfo = *parentItemInfo;
+ }
}
@@ -40,6 +47,10 @@
{
delete m_dirIterator;
delete m_mainItemInfo;
+if (m_parentItemInfo != 0)
+{
+delete m_parentItemInfo;
+}
}
@@ -47,7 +58,7 @@
{
DirItemInfoList netContent;
m_dirIterator->load();
- bool is_parent_of_smb_url = m_parent != 0 && m_parent->urlPath().startsWith(LocationUrl::SmbURL);
+ bool is_parent_of_smb_url = m_parentItemInfo != 0 && m_parentItemInfo->urlPath().startsWith(LocationUrl::SmbURL);
while (m_dirIterator->hasNext())
{
m_mainItemInfo->setFile(m_dirIterator->next());
@@ -68,7 +79,7 @@
*/
void NetworkListWorker::setSmbItemAttributes()
{
-if (m_parent->isHost()) { m_mainItemInfo->setAsShare(); }
+if (m_parentItemInfo->isHost()) { m_mainItemInfo->setAsShare(); }
else
-if (m_parent->isWorkGroup()) { m_mainItemInfo->setAsHost(); }
+if (m_parentItemInfo->isWorkGroup()) { m_mainItemInfo->setAsHost(); }
}
=== modified file 'src/plugin/folderlistmodel/networklistworker.h'
--- src/plugin/folderlistmodel/networklistworker.h 2015-12-12 14:32:18 +
+++ src/plugin/folderlistmodel/networklistworker.h 2017-01-27 12:57:59 +
@@ -40,15 +40,15 @@
public:
NetworkListWorker(LocationItemDirIterator * dirIterator,
DirItemInfo * mainItemInfo,
- const DirItemInfo * parent = 0);
+ const DirItemInfo * parentItemInfo = 0);
~NetworkListWorker();
protected:
virtual DirItemInfoList getNetworkContent();
voidsetSmbItemAttributes();
protected:
LocationItemDirIterator * m_dirIterator;
- DirItemInfo * m_mainItemInfo;
- const DirItemInfo * m_parent;
+ DirItemInfo * m_mainItemInfo; //takes ownership from mainItemInfo
+ DirItemInfo * m_parentItemInfo; //create its own instance by doing a copy from parentItemInfo
};
#endif // NETWORKLISTWORKER_H
=== modified file 'src/plugin/folderlistmodel/networklocation.cpp'
--- src/plugin/folderlistmodel/networklocation.cpp 2015-12-12 14:32:18 +
+++ src/plugin/folderlistmodel/networklocation.cpp 2017-01-27 12:57:59 +
@@ -36,6 +36,6 @@
LocationItemDirIterator *dirIterator = newDirIterator(urlPath,filter,flags,LocationItemDirIterator::LoadLater);
DirItemInfo *baseitemInfo = newItemInfo(QLatin1String(0));
-
+// the NetworkListWorker object takes
