Fix SFTP unmount

REVIEW: 129993
This commit is contained in:
Yuxuan Shui 2017-03-15 11:34:12 +01:00 committed by Albert Vaca
parent 3ed53ea469
commit f2925b7661
2 changed files with 43 additions and 26 deletions

View file

@ -54,8 +54,7 @@ Mounter::Mounter(SftpPlugin* sftp)
Mounter::~Mounter() Mounter::~Mounter()
{ {
qCDebug(KDECONNECT_PLUGIN_SFTP) << "Destroy mounter"; qCDebug(KDECONNECT_PLUGIN_SFTP) << "Destroy mounter";
//FIXME: We don't unmount becuase it crashes if we try to unmount while the filesystem is being used. Potential memory leak. unmount(false);
//unmount();
} }
bool Mounter::wait() bool Mounter::wait()
@ -64,9 +63,9 @@ bool Mounter::wait()
{ {
return true; return true;
} }
qCDebug(KDECONNECT_PLUGIN_SFTP) << "Starting loop to wait for mount"; qCDebug(KDECONNECT_PLUGIN_SFTP) << "Starting loop to wait for mount";
MountLoop loop; MountLoop loop;
connect(this, &Mounter::mounted, &loop, &MountLoop::successed); connect(this, &Mounter::mounted, &loop, &MountLoop::successed);
connect(this, &Mounter::failed, &loop, &MountLoop::failed); connect(this, &Mounter::failed, &loop, &MountLoop::failed);
@ -78,7 +77,7 @@ void Mounter::onPakcageReceived(const NetworkPackage& np)
if (np.get<bool>(QStringLiteral("stop"), false)) if (np.get<bool>(QStringLiteral("stop"), false))
{ {
qCDebug(KDECONNECT_PLUGIN_SFTP) << "SFTP server stopped"; qCDebug(KDECONNECT_PLUGIN_SFTP) << "SFTP server stopped";
unmount(); unmount(false);
return; return;
} }
@ -96,9 +95,9 @@ void Mounter::onPakcageReceived(const NetworkPackage& np)
* Q_EMIT mounted(); * Q_EMIT mounted();
*/ */
unmount(); unmount(false);
m_proc = new KProcess(this); m_proc = new KProcess();
m_proc->setOutputChannelMode(KProcess::MergedChannels); m_proc->setOutputChannelMode(KProcess::MergedChannels);
connect(m_proc, &QProcess::started, this, &Mounter::onStarted); connect(m_proc, &QProcess::started, this, &Mounter::onStarted);
@ -148,11 +147,13 @@ void Mounter::onStarted()
//m_proc->setStandardOutputFile("/tmp/kdeconnect-sftp.out"); //m_proc->setStandardOutputFile("/tmp/kdeconnect-sftp.out");
//m_proc->setStandardErrorFile("/tmp/kdeconnect-sftp.err"); //m_proc->setStandardErrorFile("/tmp/kdeconnect-sftp.err");
connect(m_proc, &KProcess::readyReadStandardError, [this]() {
qCDebug(KDECONNECT_PLUGIN_SFTP) << "stderr: " << m_proc->readAll(); auto proc = m_proc;
connect(m_proc, &KProcess::readyReadStandardError, [proc]() {
qCDebug(KDECONNECT_PLUGIN_SFTP) << "stderr: " << proc->readAll();
}); });
connect(m_proc, &KProcess::readyReadStandardOutput, [this]() { connect(m_proc, &KProcess::readyReadStandardOutput, [proc]() {
qCDebug(KDECONNECT_PLUGIN_SFTP) << "stdout:" << m_proc->readAll(); qCDebug(KDECONNECT_PLUGIN_SFTP) << "stdout:" << proc->readAll();
}); });
} }
@ -178,8 +179,8 @@ void Mounter::onFinished(int exitCode, QProcess::ExitStatus exitStatus)
qCDebug(KDECONNECT_PLUGIN_SFTP) << "Process failed (exit code:" << exitCode << ")"; qCDebug(KDECONNECT_PLUGIN_SFTP) << "Process failed (exit code:" << exitCode << ")";
Q_EMIT failed(i18n("Error when accessing to filesystem")); Q_EMIT failed(i18n("Error when accessing to filesystem"));
} }
unmount(); unmount(true);
} }
void Mounter::onMountTimeout() void Mounter::onMountTimeout()
@ -192,22 +193,38 @@ void Mounter::start()
{ {
NetworkPackage np(PACKAGE_TYPE_SFTP_REQUEST, {{"startBrowsing", true}}); NetworkPackage np(PACKAGE_TYPE_SFTP_REQUEST, {{"startBrowsing", true}});
m_sftp->sendPackage(np); m_sftp->sendPackage(np);
m_connectTimer.start(); m_connectTimer.start();
} }
void Mounter::unmount() void Mounter::unmount(bool finished)
{ {
qCDebug(KDECONNECT_PLUGIN_SFTP) << "Unmount" << m_proc; qCDebug(KDECONNECT_PLUGIN_SFTP) << "Unmount" << m_proc;
if (m_proc) if (m_proc)
{ {
auto toDestroy = m_proc; if (!finished)
m_proc = nullptr; //So we don't reenter this code path when onFinished gets called {
toDestroy->kill(); //Process is still running, we want to stop it
delete toDestroy; //But when the finished signal come, we might have already gone.
//Disconnect everything.
m_proc->disconnect();
m_proc->kill();
auto proc = m_proc;
m_proc = nullptr;
connect(proc, static_cast<void(QProcess::*)(int, QProcess::ExitStatus)>(&QProcess::finished),
[proc]() {
qCDebug(KDECONNECT_PLUGIN_SFTP) << "Free" << proc;
proc->deleteLater();
});
Q_EMIT unmounted();
}
else
m_proc->deleteLater();
//Free mount point (won't always succeed if the path is in use) //Free mount point (won't always succeed if the path is in use)
KProcess::execute(QStringList() << QStringLiteral("fusermount") << QStringLiteral("-u") << m_mountPoint, 10000); KProcess::execute(QStringList() << QStringLiteral("fusermount") << QStringLiteral("-u") << m_mountPoint, 10000);
m_proc = nullptr;
} }
m_started = false; m_started = false;
} }

View file

@ -33,18 +33,18 @@ class Mounter
{ {
Q_OBJECT Q_OBJECT
public: public:
explicit Mounter(SftpPlugin *sftp); explicit Mounter(SftpPlugin *sftp);
~Mounter() override; ~Mounter() override;
bool wait(); bool wait();
bool isMounted() const { return m_started; } bool isMounted() const { return m_started; }
Q_SIGNALS: Q_SIGNALS:
void mounted(); void mounted();
void unmounted(); void unmounted();
void failed(const QString& message); void failed(const QString& message);
private Q_SLOTS: private Q_SLOTS:
void onPakcageReceived(const NetworkPackage& np); void onPakcageReceived(const NetworkPackage& np);
void onStarted(); void onStarted();
@ -54,8 +54,8 @@ private Q_SLOTS:
void start(); void start();
private: private:
void unmount(); void unmount(bool finished);
private: private:
SftpPlugin* m_sftp; SftpPlugin* m_sftp;
KProcess* m_proc; KProcess* m_proc;