Don't redirect smb:/ to smb:// and then to smb:///
ClosedPublic

Authored by jtamate on Apr 20 2018, 8:26 AM.

Details

Summary

Append / to the url path only if the url path is empty.
This avoids a crash in KCoreDirListerCache when redirected from smb:// to smb:///.
As a side effect, it also fixes the crash I got in D10989

Test Plan

With samba started locally.
Execute kwrite
Press "save as" and go to network, then Samba shared resources. Close the dialog.
Again press "save as" and go to network, then Samba shared resources.
Previously, always crash.
Now, no error message.

Diff Detail

Repository
R241 KIO
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jtamate created this revision.Apr 20 2018, 8:26 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 20 2018, 8:26 AM
jtamate requested review of this revision.Apr 20 2018, 8:26 AM
apol added a subscriber: apol.Apr 20 2018, 9:27 AM
apol added inline comments.
src/widgets/kdirmodel.cpp
401 ↗(On Diff #32626)

Maybe KCoreDirListerCache should be using a QPointer? or checking if it gets deleted?

src/widgets/kdirmodel.h
65 ↗(On Diff #32626)

It's a bit weird having a comment after a comment.

jtamate updated this revision to Diff 32640.Apr 20 2018, 11:47 AM
jtamate edited the summary of this revision. (Show Details)
jtamate edited the test plan for this revision. (Show Details)

Implemented as Aleix suggested, using QPointer.

apol requested changes to this revision.Apr 20 2018, 12:35 PM

^^' this won't work though. If the object gets deleted you will get a null pointer in the list.

This revision now requires changes to proceed.Apr 20 2018, 12:35 PM
In D12371#250366, @apol wrote:

^^' this won't work though. If the object gets deleted you will get a null pointer in the list.

If the QPointer kdl detects that the guarded pointer is invalid it is removed from the list.

if (kdl.isNull()) {
    curHolders.removeAll(kdl);
}

I'm unable to create the crash.

dfaure requested changes to this revision.Apr 20 2018, 5:21 PM

I find this solution horrible. QPointers screams "we designed this wrongly, the ownership rules are unclear, stuff gets deleted randomly, we have to guard against it".

So while I don't deny that we might have designed this wrongly (and if we did, it's probably my fault), the right solution is to design this correctly, not to sprinkle qpointers.

Changing the documented ownership (as in the first version of the patch) isn't a solution either (that's behaviour incompatible by definition), but a standard solution is setAutoDeleteDirLister(false) for the case where you don't want the ownership to be transferred.

I don't understand "KCoreDirListerCache is probably the owner." People can create a KDirLister directly.

Also, the commit description reads like "kdirlister got deleted, KCoreDirListerCache wasn't told", which is surprising since the KCoreDirLister destructor deregisters from the cache using forgetDirs(this), which ends up calling dirData.listersCurrentlyHolding.removeAll(lister);
So I think the investigation hasn't been completed, which then leads to a bad fix...

More investigation, in bold what causes the problem.

moveListersWithoutCachedItemsJob append KDirLister(0x3b510c70) to holders for url= QUrl("file:///virtual/kde5/5kde/build/frameworks/kio")
forgetDirs removeAll in directoryData[ "file:///virtual/kde5/5kde/build/frameworks/kio" ] for KDirLister(0x3b510c70)
moveListersWithoutCachedItemsJob append KDirLister(0x3b510c70) to holders for url= QUrl("remote:/")
forgetDirs removeAll in directoryData[ "remote:/" ] for KDirLister(0x3b510c70)
forgetDirs not found: "smb://" in directoryData for KDirLister(0x3b510c70)
moveListersWithoutCachedItemsJob append KDirLister(0x3b510c70) to holders for url= QUrl("smb:///")
moveListersWithoutCachedItemsJob append KDirLister(0x3b510c70) to holders for url= QUrl("smb://")
forgetDirs removeAll in directoryData[ "smb:///" ] for KDirLister(0x3b510c70)
moveListersWithoutCachedItemsJob append KDirLister(0x3b510c70) to holders for url= QUrl("smb://127.0.0.1/")
forgetDirs removeAll in directoryData[ "smb://127.0.0.1/" ] for KDirLister(0x3b510c70)
moveListersWithoutCachedItemsJob append KDirLister(0x3b510c70) to holders for url= QUrl("smb://127.0.0.1/kk")
forgetDirs removeAll in directoryData[ "smb://127.0.0.1/kk" ] for KCoreDirLister(0x3b510c70)
forgetCachedItemsJob append KDirLister(0x4e273400) to holders for url= QUrl("smb://127.0.0.1/kk")
forgetDirs removeAll in directoryData[ "smb://127.0.0.1/kk" ] for KDirLister(0x4e273400)
forgetCachedItemsJob append KDirLister(0x4e273400) to holders for url= QUrl("file:///virtual/kde5/5kde/build/frameworks/kio")
forgetDirs removeAll in directoryData[ "file:///virtual/kde5/5kde/build/frameworks/kio" ] for KDirLister(0x4e273400)
moveListersWithoutCachedItemsJob append KDirLister(0x4e273400) to holders for url= QUrl("smb://127.0.0.1/kk")
forgetDirs removeAll in directoryData[ "smb://127.0.0.1/kk" ] for KDirLister(0x4e273400)
forgetCachedItemsJob append KDirLister(0x4e273400) to holders for url= QUrl("file:///virtual/kde5/5kde/build/frameworks/kio")
forgetDirs removeAll in directoryData[ "file:///virtual/kde5/5kde/build/frameworks/kio" ] for KDirLister(0x4e273400)
forgetCachedItemsJob append KDirLister(0x4e273400) to holders for url= QUrl("file:///virtual/kde5/5kde/build/frameworks/kio")
forgetDirs removeAll in directoryData[ "file:///virtual/kde5/5kde/build/frameworks/kio" ] for KDirLister(0x4e273400)
forgetCachedItemsJob append KDirLister(0x4e273400) to holders for url= QUrl("remote:/")
forgetDirs removeAll in directoryData[ "remote:/" ] for KDirLister(0x4e273400)
forgetDirs removeAll in directoryData[ "smb://" ] for KDirLister(0x4e273400)
number of holders for "smb://" 1

==23870== Invalid read of size 8
==23870==    at 0x5F41167: KCoreDirListerCache::slotRedirection(KIO::Job*, QUrl const&) (kcoredirlister.cpp:1484)
==23870==    by 0x5F4A3A9: call (qobjectdefs_impl.h:136)
==23870==    by 0x5F4A3A9: call<QtPrivate::List<KIO::Job*, const QUrl&>, void> (qobjectdefs_impl.h:169)
==23870==    by 0x5F4A3A9: QtPrivate::QSlotObject<void (KCoreDirListerCache::*)(KIO::Job*, QUrl const&), QtPrivate::List<KIO::Job*, QUrl const&>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (qobjectdefs_impl.h:398)
==23870==    by 0xA0F736B: call (qobjectdefs_impl.h:378)
==23870==    by 0xA0F736B: QMetaObject::activate(QObject*, int, int, void**) (qobject.cpp:3749)
==23870==    by 0x5EED068: KIO::ListJob::redirection(KIO::Job*, QUrl const&) (moc_listjob.cpp:246)
==23870==    by 0x5EED481: KIO::ListJobPrivate::slotRedirection(QUrl const&) (listjob.cpp:222)
==23870==    by 0x5EED4C1: operator() (listjob.cpp:294)
==23870==    by 0x5EED4C1: call (qobjectdefs_impl.h:130)
==23870==    by 0x5EED4C1: call<QtPrivate::List<const QUrl&>, void> (qobjectdefs_impl.h:240)
==23870==    by 0x5EED4C1: QtPrivate::QFunctorSlotObject<KIO::ListJobPrivate::start(KIO::Slave*)::{lambda(QUrl const&)#4}, 1, QtPrivate::List<QUrl const&>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (qobjectdefs_impl.h:423)
==23870==    by 0xA0F736B: call (qobjectdefs_impl.h:378)
==23870==    by 0xA0F736B: QMetaObject::activate(QObject*, int, int, void**) (qobject.cpp:3749)
==23870==    by 0x5ED159A: KIO::SlaveInterface::redirection(QUrl const&) (moc_slaveinterface.cpp:518)
==23870==    by 0x5ED3B39: KIO::SlaveInterface::dispatch(int, QByteArray const&) (slaveinterface.cpp:248)
==23870==    by 0x5ED16F5: KIO::SlaveInterface::dispatch() (slaveinterface.cpp:89)
==23870==    by 0x5ED64A6: KIO::Slave::gotInput() (slave.cpp:406)
==23870==    by 0x5F660AB: KIO::Slave::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (moc_slave.cpp:89)
==23870==  Address 0x3b510c70 is 0 bytes inside a block of size 32 free'd
==23870==    at 0x4C2F7BB: operator delete(void*) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==23870==    by 0x5BD45B6: KDirLister::~KDirLister() (kdirlister.cpp:55)
==23870==    by 0xA0F503A: QObjectPrivate::deleteChildren() (qobject.cpp:1992)
==23870==    by 0xA0FE38A: QObject::~QObject() (qobject.cpp:1022)
==23870==    by 0x5BD6E68: KDirModel::~KDirModel() (kdirmodel.cpp:394)
==23870==    by 0x5BD6E78: KDirModel::~KDirModel() (kdirmodel.cpp:397)
==23870==    by 0x210252F9: ??? (in /usr/lib64/libKF5KIOFileWidgets.so.5.44.0)
==23870==    by 0x21025492: KDirOperator::~KDirOperator() (in /usr/lib64/libKF5KIOFileWidgets.so.5.44.0)
==23870==    by 0x210254C8: KDirOperator::~KDirOperator() (in /usr/lib64/libKF5KIOFileWidgets.so.5.44.0)
==23870==    by 0x2103C33B: KFileWidget::~KFileWidget() (in /usr/lib64/libKF5KIOFileWidgets.so.5.44.0)
==23870==    by 0x2103C4B8: KFileWidget::~KFileWidget() (in /usr/lib64/libKF5KIOFileWidgets.so.5.44.0)
==23870==    by 0xA0F503A: QObjectPrivate::deleteChildren() (qobject.cpp:1992)
==23870==  Block was alloc'd at
==23870==    at 0x4C2E6FF: operator new(unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==23870==    by 0x21028589: KDirOperator::KDirOperator(QUrl const&, QWidget*) (in /usr/lib64/libKF5KIOFileWidgets.so.5.44.0)
==23870==    by 0x2103EB77: KFileWidget::KFileWidget(QUrl const&, QWidget*) (in /usr/lib64/libKF5KIOFileWidgets.so.5.44.0)
==23870==    by 0x20B722CD: ??? (in /usr/lib64/qt5/plugins/platformthemes/KDEPlasmaPlatformTheme.so)
==23870==    by 0x20B728A4: ??? (in /usr/lib64/qt5/plugins/platformthemes/KDEPlasmaPlatformTheme.so)
==23870==    by 0x20B67609: ??? (in /usr/lib64/qt5/plugins/platformthemes/KDEPlasmaPlatformTheme.so)
==23870==    by 0x8F06654: QDialogPrivate::platformHelper() const (qdialog.cpp:122)
==23870==    by 0x8F1755A: platformFileDialogHelper (qfiledialog_p.h:122)
==23870==    by 0x8F1755A: QFileDialogPrivate::init(QUrl const&, QString const&, QString const&) (qfiledialog.cpp:2826)
==23870==    by 0x8F17995: QFileDialog::QFileDialog(QFileDialogArgs const&) (qfiledialog.cpp:373)
==23870==    by 0x8F18311: QFileDialog::getSaveFileUrl(QWidget*, QString const&, QUrl const&, QString const&, QString*, QFlags<QFileDialog::Option>, QStringList const&) (qfiledialog.cpp:2413)
==23870==    by 0x4F5877E: KTextEditor::DocumentPrivate::documentSaveAs() (katedocument.cpp:4314)
==23870==    by 0x50F868D: KTextEditor::DocumentPrivate::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (moc_katedocument.cpp:514)
==23870== 
KCrash: crashing... crashRecursionCounter = 2

So while I don't deny that we might have designed this wrongly (and if we did, it's probably my fault), the right solution is to design this correctly, not to sprinkle qpointers.

It is well designed. But I'll include some comments, just in case someone else gets lost in the forest as I did.

The problem seems to be in moveListersWithoutCachedItemsJob or in forgetDirs.

It looks like the issue is that forgetDirs() doesn't find smb:// on the line of code that says DirectoryDataHash::iterator dit = directoryData.find(urlStr);.
So it goes into early return and doesn't do dirData.listersCurrentlyHolding.removeAll(lister);
which later leads to using a deleted lister that is still in listersCurrentlyHolding (for a slightly different URL).

Can you check the contents of directoryData at that point? Could it be that it has an entry for smb:/// with three slashes?
One solution is to call printDebug(), which will output lots of information including the contents of directoryData().

One solution is to call printDebug(), which will output lots of information including the contents of directoryData().

I have some questions:
Is a KCoreDirLister able to handle more than one directory? The same KCoreDirLister pointer is holding smb:// and smb:/// at some point.
Shouldn't something be done in the cache when the KCoreDirLister changes from smb:// to smb:/// ?

The output I get, with a printDebug() call before Iterating over dirs;

kf5.kio.core.dirlister: +KCoreDirLister
kf5.kio.core.dirlister: 
kf5.kio.core.dirlister: +KCoreDirLister
kf5.kio.core.dirlister: ~KCoreDirLister KCoreDirLister(0x2b6add0)
kf5.kio.core.dirlister: lister: KCoreDirLister(0x2b6add0) silent= false
kf5.kio.core.dirlister: KCoreDirLister(0x2b6add0)
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: Iterating over dirs ()
kf5.kio.core.dirlister: KDirLister(0x2b69460) url= QUrl("file:///kk") keep= false reload= false
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: lister: KDirLister(0x2b69460) silent= true
kf5.kio.core.dirlister: KDirLister(0x2b69460)
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: Iterating over dirs ()
kf5.kio.core.dirlister: Listing directory: QUrl("file:///kk")
kf5.kio.core.dirlister: Entry now being listed by (KDirLister(0x2b69460))
kf5.kio.core.dirlister: new entries for  QUrl("file:///kk")
kf5.kio.core.dirlister: finished listing QUrl("file:///kk")
kf5.kio.core.dirlister: KDirLister(0x2b69460) numJobs: 0
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "file:///kk" URL: QUrl("file:///kk") rootItem: QUrl("file:///kk") autoUpdates refcount: 1 complete: true "with 0 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "file:///kk" 0 listers: ""
kf5.kio.core.dirlister:    "file:///kk" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: KDirLister(0x2b69460) url= QUrl("remote:/") keep= false reload= false
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "file:///kk" URL: QUrl("file:///kk") rootItem: QUrl("file:///kk") autoUpdates refcount: 1 complete: true "with 0 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "file:///kk" 0 listers: ""
kf5.kio.core.dirlister:    "file:///kk" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: lister: KDirLister(0x2b69460) silent= true
kf5.kio.core.dirlister: KDirLister(0x2b69460)  url= QUrl("file:///kk")
kf5.kio.core.dirlister: KDirLister(0x2b69460)
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "file:///kk" URL: QUrl("file:///kk") rootItem: QUrl("file:///kk") autoUpdates refcount: 1 complete: true "with 0 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "file:///kk" 0 listers: ""
kf5.kio.core.dirlister:    "file:///kk" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: Iterating over dirs (QUrl("file:///kk"))
kf5.kio.core.dirlister: KDirLister(0x2b69460)  _url:  QUrl("file:///kk")
kf5.kio.core.dirlister: KDirLister(0x2b69460) item moved into cache: QUrl("file:///kk")
kf5.kio.core.dirlister: Listing directory: QUrl("remote:/")
kf5.kio.core.dirlister: Entry now being listed by (KDirLister(0x2b69460))
kf5.kio.core.dirlister: new entries for  QUrl("remote:/")
kf5.kio.core.dirlister: Adding item:  QUrl("remote:/x-wizard_service.desktop")
kf5.kio.core.dirlister: in QUrl("remote:/") item: QUrl("remote:/x-wizard_service.desktop")
kf5.kio.core.dirlister: Adding item:  QUrl("remote:/bluetooth-network")
kf5.kio.core.dirlister: in QUrl("remote:/") item: QUrl("remote:/bluetooth-network")
kf5.kio.core.dirlister: Adding item:  QUrl("remote:/mtp-network")
kf5.kio.core.dirlister: in QUrl("remote:/") item: QUrl("remote:/mtp-network")
kf5.kio.core.dirlister: Adding item:  QUrl("remote:/network")
kf5.kio.core.dirlister: in QUrl("remote:/") item: QUrl("remote:/network")
kf5.kio.core.dirlister: Adding item:  QUrl("remote:/smb-network")
kf5.kio.core.dirlister: in QUrl("remote:/") item: QUrl("remote:/smb-network")
kf5.kio.core.dirlister: Adding item:  QUrl("remote:/zeroconf")
kf5.kio.core.dirlister: in QUrl("remote:/") item: QUrl("remote:/zeroconf")
kf5.kio.core.dirlister: finished listing QUrl("remote:/")
kf5.kio.core.dirlister: KDirLister(0x2b69460) numJobs: 0
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "remote:/" URL: QUrl("remote:/") rootItem: QUrl("remote:/") autoUpdates refcount: 1 complete: true "with 6 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "remote:/" 0 listers: ""
kf5.kio.core.dirlister:    "remote:/" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "file:///kk" rootItem: "file:///kk" with 0 items.
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: KDirLister(0x2b69460) url= QUrl("smb:/") keep= false reload= false
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "remote:/" URL: QUrl("remote:/") rootItem: QUrl("remote:/") autoUpdates refcount: 1 complete: true "with 6 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "remote:/" 0 listers: ""
kf5.kio.core.dirlister:    "remote:/" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "file:///kk" rootItem: "file:///kk" with 0 items.
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: lister: KDirLister(0x2b69460) silent= true
kf5.kio.core.dirlister: KDirLister(0x2b69460)  url= QUrl("remote:/")
kf5.kio.core.dirlister: KDirLister(0x2b69460)
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "remote:/" URL: QUrl("remote:/") rootItem: QUrl("remote:/") autoUpdates refcount: 1 complete: true "with 6 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "remote:/" 0 listers: ""
kf5.kio.core.dirlister:    "remote:/" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "file:///kk" rootItem: "file:///kk" with 0 items.
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: Iterating over dirs (QUrl("remote:/"))
kf5.kio.core.dirlister: KDirLister(0x2b69460)  _url:  QUrl("remote:/")
kf5.kio.core.dirlister: KDirLister(0x2b69460) item moved into cache: QUrl("remote:/")
kf5.kio.core.dirlister: Listing directory: QUrl("smb:/")
kf5.kio.core.dirlister: Entry now being listed by (KDirLister(0x2b69460))
kf5.kio.core.dirlister: QUrl("smb:/") -> QUrl("smb://")
kf5.kio.core.dirlister: KDirLister(0x2b69460) url= QUrl("smb:///") keep= false reload= false
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister:     KIO::ListJob(0x2cf6a90) listing QUrl("smb://") : 0 entries.
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "file:///kk" rootItem: "file:///kk" with 0 items.
kf5.kio.core.dirlister:     "remote:/" rootItem: "remote:/" with 6 items.
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: lister: KDirLister(0x2b69460) silent= true
kf5.kio.core.dirlister: KDirLister(0x2b69460)  url= QUrl("smb://")
kf5.kio.core.dirlister: KDirLister(0x2b69460)
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister:     KIO::ListJob(0x2cf6a90) listing QUrl("smb://") : 0 entries.
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "file:///kk" rootItem: "file:///kk" with 0 items.
kf5.kio.core.dirlister:     "remote:/" rootItem: "remote:/" with 6 items.
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: Iterating over dirs (QUrl("smb://"))
kf5.kio.core.dirlister: KDirLister(0x2b69460)  _url:  QUrl("smb://")
kf5.kio.core.dirlister: Listing directory: QUrl("smb:///")
kf5.kio.core.dirlister: Entry now being listed by (KDirLister(0x2b69460))
kf5.kio.core.dirlister: QUrl("smb://") has not been listed yet.
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "smb://" URL: QUrl("smb://") rootItem: QUrl("") autoUpdates refcount: 1 complete: false "with 0 items."
kf5.kio.core.dirlister:     "smb:///" URL: QUrl("smb:///") rootItem: QUrl("") autoUpdates refcount: 1 complete: false "with 0 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "smb://" 1 listers: " 0x2b69460"
kf5.kio.core.dirlister:   Lister KDirLister(0x2b69460) has ListJob KIO::ListJob(0x2cf6a90)
kf5.kio.core.dirlister:    "smb://" 0 holders: ""
kf5.kio.core.dirlister:    "smb:///" 1 listers: " 0x2b69460"
kf5.kio.core.dirlister:   Lister KDirLister(0x2b69460) has ListJob KIO::ListJob(0x2e1ce20)
kf5.kio.core.dirlister:    "smb:///" 0 holders: ""
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister:     KIO::ListJob(0x2cf6a90) listing QUrl("smb://") : 0 entries.
kf5.kio.core.dirlister:     KIO::ListJob(0x2e1ce20) listing QUrl("smb:///") : 0 entries.
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "file:///kk" rootItem: "file:///kk" with 0 items.
kf5.kio.core.dirlister:     "remote:/" rootItem: "remote:/" with 6 items.
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: new entries for  QUrl("smb://")
kf5.kio.core.dirlister: finished listing QUrl("smb://")
kf5.kio.core.dirlister: KDirLister(0x2b69460) numJobs: 0
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "smb://" URL: QUrl("smb://") rootItem: QUrl("smb://") autoUpdates refcount: 1 complete: true "with 0 items."
kf5.kio.core.dirlister:     "smb:///" URL: QUrl("smb:///") rootItem: QUrl("") autoUpdates refcount: 1 complete: false "with 0 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "smb://" 0 listers: ""
kf5.kio.core.dirlister:    "smb://" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister:    "smb:///" 1 listers: " 0x2b69460"
kf5.kio.core.dirlister:   Lister KDirLister(0x2b69460) has ListJob KIO::ListJob(0x2e1ce20)
kf5.kio.core.dirlister:    "smb:///" 0 holders: ""
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister:     KIO::ListJob(0x2e1ce20) listing QUrl("smb:///") : 0 entries.
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "file:///kk" rootItem: "file:///kk" with 0 items.
kf5.kio.core.dirlister:     "remote:/" rootItem: "remote:/" with 6 items.
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: finished listing QUrl("smb:///")
kf5.kio.core.dirlister: KDirLister(0x2b69460) numJobs: 0
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "smb://" URL: QUrl("smb://") rootItem: QUrl("smb://") autoUpdates refcount: 1 complete: true "with 0 items."
kf5.kio.core.dirlister:     "smb:///" URL: QUrl("smb:///") rootItem: QUrl("") autoUpdates refcount: 1 complete: false "with 0 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "smb://" 0 listers: ""
kf5.kio.core.dirlister:    "smb://" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister:    "smb:///" 0 listers: ""
kf5.kio.core.dirlister:    "smb:///" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "file:///kk" rootItem: "file:///kk" with 0 items.
kf5.kio.core.dirlister:     "remote:/" rootItem: "remote:/" with 6 items.
kf5.kio.core.dirlister: =========
Aborting aboutToFinish handling.
kf5.kio.core.dirlister: lister: KDirLister(0x2b69460) silent= false
kf5.kio.core.dirlister: KDirLister(0x2b69460)  url= QUrl("smb:///")
kf5.kio.core.dirlister: ~KCoreDirLister KCoreDirLister(0x2b69460)
kf5.kio.core.dirlister: lister: KCoreDirLister(0x2b69460) silent= false
kf5.kio.core.dirlister: KCoreDirLister(0x2b69460)  url= QUrl("smb:///")
kf5.kio.core.dirlister: KCoreDirLister(0x2b69460)
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "smb://" URL: QUrl("smb://") rootItem: QUrl("smb://") autoUpdates refcount: 1 complete: true "with 0 items."
kf5.kio.core.dirlister:     "smb:///" URL: QUrl("smb:///") rootItem: QUrl("") autoUpdates refcount: 1 complete: false "with 0 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "smb://" 0 listers: ""
kf5.kio.core.dirlister:    "smb://" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister:    "smb:///" 0 listers: ""
kf5.kio.core.dirlister:    "smb:///" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "file:///kk" rootItem: "file:///kk" with 0 items.
kf5.kio.core.dirlister:     "remote:/" rootItem: "remote:/" with 6 items.
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: Iterating over dirs (QUrl("smb:///"))
kf5.kio.core.dirlister: KCoreDirLister(0x2b69460)  _url:  QUrl("smb:///")
kf5.kio.core.dirlister: +KCoreDirLister
kf5.kio.core.dirlister: +KCoreDirLister
kf5.kio.core.dirlister: ~KCoreDirLister KCoreDirLister(0x2c68290)
kf5.kio.core.dirlister: lister: KCoreDirLister(0x2c68290) silent= false
kf5.kio.core.dirlister: KCoreDirLister(0x2c68290)
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "smb://" URL: QUrl("smb://") rootItem: QUrl("smb://") autoUpdates refcount: 1 complete: true "with 0 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "smb://" 0 listers: ""
kf5.kio.core.dirlister:    "smb://" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "file:///kk" rootItem: "file:///kk" with 0 items.
kf5.kio.core.dirlister:     "remote:/" rootItem: "remote:/" with 6 items.
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: Iterating over dirs ()
kf5.kio.core.dirlister: KDirLister(0x1d31640) url= QUrl("file:///kk") keep= false reload= false
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "smb://" URL: QUrl("smb://") rootItem: QUrl("smb://") autoUpdates refcount: 1 complete: true "with 0 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "smb://" 0 listers: ""
kf5.kio.core.dirlister:    "smb://" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "file:///kk" rootItem: "file:///kk" with 0 items.
kf5.kio.core.dirlister:     "remote:/" rootItem: "remote:/" with 6 items.
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: lister: KDirLister(0x1d31640) silent= true
kf5.kio.core.dirlister: KDirLister(0x1d31640)
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "smb://" URL: QUrl("smb://") rootItem: QUrl("smb://") autoUpdates refcount: 1 complete: true "with 0 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "smb://" 0 listers: ""
kf5.kio.core.dirlister:    "smb://" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "file:///kk" rootItem: "file:///kk" with 0 items.
kf5.kio.core.dirlister:     "remote:/" rootItem: "remote:/" with 6 items.
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: Iterating over dirs ()
kf5.kio.core.dirlister: Entry in cache: QUrl("file:///kk")
kf5.kio.core.dirlister: Creating CachedItemsJob KCoreDirLister::Private::CachedItemsJob(0x31c8040) for lister KDirLister(0x1d31640) QUrl("file:///kk")
kf5.kio.core.dirlister: Moving from listing to holding, because no more job KDirLister(0x1d31640) "file:///kk"
kf5.kio.core.dirlister: KDirLister(0x1d31640) url= QUrl("remote:/") keep= false reload= false
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "file:///kk" URL: QUrl("file:///kk") rootItem: QUrl("file:///kk") autoUpdates refcount: 1 complete: true "with 0 items."
kf5.kio.core.dirlister:     "smb://" URL: QUrl("smb://") rootItem: QUrl("smb://") autoUpdates refcount: 1 complete: true "with 0 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "file:///kk" 0 listers: ""
kf5.kio.core.dirlister:    "file:///kk" 1 holders: " 0x1d31640"
kf5.kio.core.dirlister:    "smb://" 0 listers: ""
kf5.kio.core.dirlister:    "smb://" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "remote:/" rootItem: "remote:/" with 6 items.
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: lister: KDirLister(0x1d31640) silent= true
kf5.kio.core.dirlister: KDirLister(0x1d31640)  url= QUrl("file:///kk")
kf5.kio.core.dirlister: KDirLister(0x1d31640)
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "file:///kk" URL: QUrl("file:///kk") rootItem: QUrl("file:///kk") autoUpdates refcount: 1 complete: true "with 0 items."
kf5.kio.core.dirlister:     "smb://" URL: QUrl("smb://") rootItem: QUrl("smb://") autoUpdates refcount: 1 complete: true "with 0 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "file:///kk" 0 listers: ""
kf5.kio.core.dirlister:    "file:///kk" 1 holders: " 0x1d31640"
kf5.kio.core.dirlister:    "smb://" 0 listers: ""
kf5.kio.core.dirlister:    "smb://" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "remote:/" rootItem: "remote:/" with 6 items.
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: Iterating over dirs (QUrl("file:///kk"))
kf5.kio.core.dirlister: KDirLister(0x1d31640)  _url:  QUrl("file:///kk")
kf5.kio.core.dirlister: KDirLister(0x1d31640) item moved into cache: QUrl("file:///kk")
kf5.kio.core.dirlister: Entry in cache: QUrl("remote:/")
kf5.kio.core.dirlister: Creating CachedItemsJob KCoreDirLister::Private::CachedItemsJob(0x3138080) for lister KDirLister(0x1d31640) QUrl("remote:/")
kf5.kio.core.dirlister: emitting 6 for lister KDirLister(0x1d31640)
kf5.kio.core.dirlister: in QUrl("remote:/") item: QUrl("remote:/x-wizard_service.desktop")
kf5.kio.core.dirlister: in QUrl("remote:/") item: QUrl("remote:/bluetooth-network")
kf5.kio.core.dirlister: in QUrl("remote:/") item: QUrl("remote:/mtp-network")
kf5.kio.core.dirlister: in QUrl("remote:/") item: QUrl("remote:/network")
kf5.kio.core.dirlister: in QUrl("remote:/") item: QUrl("remote:/smb-network")
kf5.kio.core.dirlister: in QUrl("remote:/") item: QUrl("remote:/zeroconf")
kf5.kio.core.dirlister: Moving from listing to holding, because no more job KDirLister(0x1d31640) "remote:/"
kf5.kio.core.dirlister: KDirLister(0x1d31640) url= QUrl("smb:/") keep= false reload= false
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "smb://" URL: QUrl("smb://") rootItem: QUrl("smb://") autoUpdates refcount: 1 complete: true "with 0 items."
kf5.kio.core.dirlister:     "remote:/" URL: QUrl("remote:/") rootItem: QUrl("remote:/") autoUpdates refcount: 1 complete: true "with 6 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "smb://" 0 listers: ""
kf5.kio.core.dirlister:    "smb://" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister:    "remote:/" 0 listers: ""
kf5.kio.core.dirlister:    "remote:/" 1 holders: " 0x1d31640"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "file:///kk" rootItem: "file:///kk" with 0 items.
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: lister: KDirLister(0x1d31640) silent= true
kf5.kio.core.dirlister: KDirLister(0x1d31640)  url= QUrl("remote:/")
kf5.kio.core.dirlister: KDirLister(0x1d31640)
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "smb://" URL: QUrl("smb://") rootItem: QUrl("smb://") autoUpdates refcount: 1 complete: true "with 0 items."
kf5.kio.core.dirlister:     "remote:/" URL: QUrl("remote:/") rootItem: QUrl("remote:/") autoUpdates refcount: 1 complete: true "with 6 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "smb://" 0 listers: ""
kf5.kio.core.dirlister:    "smb://" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister:    "remote:/" 0 listers: ""
kf5.kio.core.dirlister:    "remote:/" 1 holders: " 0x1d31640"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "file:///kk" rootItem: "file:///kk" with 0 items.
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: Iterating over dirs (QUrl("remote:/"))
kf5.kio.core.dirlister: KDirLister(0x1d31640)  _url:  QUrl("remote:/")
kf5.kio.core.dirlister: KDirLister(0x1d31640) item moved into cache: QUrl("remote:/")
kf5.kio.core.dirlister: Listing directory: QUrl("smb:/")
kf5.kio.core.dirlister: Entry now being listed by (KDirLister(0x1d31640))
kf5.kio.core.dirlister: QUrl("smb:/") -> QUrl("smb://")
kf5.kio.core.dirlister: KDirLister(0x1d31640) url= QUrl("smb:///") keep= false reload= false
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "smb://" URL: QUrl("smb://") rootItem: QUrl("smb://") autoUpdates refcount: 1 complete: true "with 0 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "smb://" 0 listers: ""
kf5.kio.core.dirlister:    "smb://" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister:     KIO::ListJob(0x3250d70) listing QUrl("smb://") : 0 entries.
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "file:///kk" rootItem: "file:///kk" with 0 items.
kf5.kio.core.dirlister:     "remote:/" rootItem: "remote:/" with 6 items.
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: lister: KDirLister(0x1d31640) silent= true
kf5.kio.core.dirlister: KDirLister(0x1d31640)  url= QUrl("smb://")
kf5.kio.core.dirlister: KDirLister(0x1d31640)
kf5.kio.core.dirlister: =========Items in use:
kf5.kio.core.dirlister:     "smb://" URL: QUrl("smb://") rootItem: QUrl("smb://") autoUpdates refcount: 1 complete: true "with 0 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "smb://" 0 listers: ""
kf5.kio.core.dirlister:    "smb://" 1 holders: " 0x2b69460"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister:     KIO::ListJob(0x3250d70) listing QUrl("smb://") : 0 entries.
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "file:///kk" rootItem: "file:///kk" with 0 items.
kf5.kio.core.dirlister:     "remote:/" rootItem: "remote:/" with 6 items.
kf5.kio.core.dirlister: =========
kf5.kio.core.dirlister: Iterating over dirs (QUrl("smb://"))
kf5.kio.core.dirlister: KDirLister(0x1d31640)  _url:  QUrl("smb://")
kf5.kio.core.dirlister: Listing directory: QUrl("smb:///")
kf5.kio.core.dirlister: Entry now being listed by (KDirLister(0x1d31640))
kf5.kio.core.dirlister: QUrl("smb://") already in use
kf5.kio.core.dirlister: and it is currently held.
KCrash: crashing... crashRecursionCounter = 2
jtamate updated this revision to Diff 33045.Apr 25 2018, 8:06 AM
jtamate edited the test plan for this revision. (Show Details)

Just to know if I'm in the correct path, because I'm getting out of ideas.
Use in the hash table, and probably in other parts, always the url with at most one trailing /.

Pros: The crash is gone.
Cons: The error handling is gone (with this partial patch). Four asserts are hit.

Can you verify who calls lister with file:///kk -> this is not correct it should be file://kk (2 slashes) then Cache will start to work again.

Anthony: do you mean smb:// ? file:/// with 3 slashes is most definitely correct (file://kk would mean a hostname of "kk"). And file://kk doesn't appear anywhere in the output, so surely there's no problem there. The question is which piece of code confuses smb:// and smb:///, do both of these URLs enter KCoreDirLister externally?

Jaime: yes KCoreDirLister can hold multiple URLs, think of tree views.

The problem is, I forgot again how we handle smb URLs. I see in your logs a redirection from smb:/ to smb://, but I don't know where smb:/// comes from and whether it's correct at all. It does seem strange, for smb. I think that's where the problem is. The kioslave seems to want it to be smb:// with 2 slashes.

After the setHost and the setPath, it'll be smb://host/path, so that one doesn't have any effect (unless both host and path are empty). And this is in the special() command so it doesn't get into the app's KCoreDirLister.

So, it should be Dolphin even empty path + '/' will result in error? And in other review concatPath can make it, what you think?

This is unrelated to concatPaths because concatPaths is about absolute+relative.
smb:// + / isn't about adding a relative path.

But yes, please find out where smb:/// comes from, and then we can discuss solutions ;)

(Check how kioclient5 ls smb:// and kioclient5 ls smb:/// show different things)

jtamate updated this revision to Diff 33211.Apr 27 2018, 4:45 PM
jtamate edited the summary of this revision. (Show Details)
jtamate edited the test plan for this revision. (Show Details)

The responsible for changing smb:// into smb:/// was: void KFileWidgetPrivate::_k_enterUrl(const QUrl &url)
With this patch, the crashes are gone, as there is no redirection to smb:///
But now there is no smb error message.

(in spanish). unless smb:/// is introduced manually as the url.

dfaure requested changes to this revision.Apr 28 2018, 10:14 AM
dfaure added inline comments.
src/filewidgets/kfilewidget.cpp
1523

This needs to be updated btw, to "because it uses QUrl::adjusted(QUrl::RemoveFilename)", but I think the idea remains the same.

1525

I don't think this is correct, we still need that code to be called for ftp:// and similar.

I think this needs to be if (!u.path().isEmpty()) so that only smb:// and ftp://host (which is supposed to redirect to the home dir) are untouched, and everything else gets a trailing slash.

Thanks for the investigation.

This revision now requires changes to proceed.Apr 28 2018, 10:14 AM
jtamate updated this revision to Diff 33242.Apr 28 2018, 11:17 AM
jtamate edited the summary of this revision. (Show Details)

If I use if (!u.path().isEmpty()) or if (!u.path().isEmpty() && !u.path().endsWith('/')),
as soon as I enter the url fish://127.0.0.1 (without trailing /), I'm redirected to fish://127.0.0.1/home/jtorres (that didn't happen before), and after finishing reading the directory, assertion.

kf5.kio.core: Internal error: job is listing QUrl("fish://127.0.0.1/home/jtorres") but directoryData says no listers are currently listing  "fish://127.0.0.1/home/jtorres"
kf5.kio.core.dirlister: Items in use:
kf5.kio.core.dirlister:     "fish://127.0.0.1/home/jtorres" URL: QUrl("fish://127.0.0.1/home/jtorres") rootItem: QUrl("fish://127.0.0.1/home/jtorres") autoUpdates refcount: 1 complete: true "with 1010 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:    "fish://127.0.0.1/home/jtorres" 0 listers: ""
kf5.kio.core.dirlister:    "fish://127.0.0.1/home/jtorres" 1 holders: " 0x3279dd0"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister:     "file:///virtual/kde5/5kde/build/frameworks/kio" rootItem: "file:///virtual/kde5/5kde/build/frameworks/kio" with 16 items.
ASSERT: "!dirData.listersCurrentlyListing.isEmpty()" in file /g/5kde/frameworks/kio/src/core/kcoredirlister.cpp, line 1208


#9  QMessageLogger::fatal (this=this@entry=0x7fff05486530, msg=msg@entry=0x7f36ba7ddeb0 "ASSERT: \"%s\" in file %s, line %d") at global/qlogging.cpp:816
#10 0x00007f36ba548e86 in qt_assert (assertion=assertion@entry=0x7f36beb7b410 "!dirData.listersCurrentlyListing.isEmpty()", file=file@entry=0x7f36beb7b0b0 "/g/5kde/frameworks/kio/src/core/kcoredirlister.cpp", line=line@entry=1208) at global/qglobal.cpp:3123
#11 0x00007f36beb40230 in KCoreDirListerCache::slotEntries (this=0x7f36bedaf420 <(anonymous namespace)::Q_QGS_kDirListerCache::innerFunction()::holder>, job=<optimized out>, entries=...) at /g/5kde/frameworks/kio/src/core/kcoredirlister.cpp:1208
...
#18 0x00007f36beae500d in KIO::ListJob::entries (this=this@entry=0x324dd20, _t1=<optimized out>, _t1@entry=0x324dd20, _t2=...) at /virtual/kde5/5kde/build/frameworks/kio/src/core/KF5KIOCore_autogen/include/moc_listjob.cpp:232
#19 0x00007f36beae6ce2 in KIO::ListJobPrivate::slotListEntries (this=0x37bd250, list=...) at /g/5kde/frameworks/kio/src/core/listjob.cpp:154
#20 0x00007f36beae7052 in KIO::ListJobPrivate::<lambda(const UDSEntryList&)>::operator() (list=..., __closure=<optimized out>) at /g/5kde/frameworks/kio/src/core/listjob.cpp:288

Checking for the slash in the string representation of the url, not only in the path part, works for me.

"I'm redirected to fish://127.0.0.1/home/jtorres" is exactly what is supposed to happen. If that asserts nowadays, it means there's a regression in the redirect handling. Sounds like this has to be fixed first.

QUrl::toString is slowish btw, it has to assemble the string from the multiple components of the URLs. Just to test the path, it's overkill.

Anyhow, "if (!u.path().isEmpty()) " is what makes sense, we "just" have to fix redirection handling too (I can take a look tomorrow, I have to leave soon today)

jtamate updated this revision to Diff 33872.May 9 2018, 9:21 AM
jtamate marked an inline comment as done.
jtamate edited the summary of this revision. (Show Details)

The crash after the redirection in fish://127.0.0.1 was due to a remaining of an unfinished patch in other files.
After removing it and testing the patch in different machines and different compilers, this solves this crash and I haven't noticed regressions.

Restricted Application added a subscriber: kde-frameworks-devel. · View Herald TranscriptMay 9 2018, 9:21 AM
dfaure accepted this revision.May 9 2018, 9:57 AM

Patch looks good, but please update the first line of the commit message so that it gives more context (think about the changelog, where the rest of the description won't be visible...)

jtamate retitled this revision from fix always reproducible crash to Don't redirect smb:/ to smb:// and then to smb:///.May 10 2018, 5:22 PM
jtamate edited the summary of this revision. (Show Details)
This revision was not accepted when it landed; it landed in state Needs Review.May 10 2018, 5:22 PM
This revision was automatically updated to reflect the committed changes.