Commit 0661fd6f authored by Max Kellermann's avatar Max Kellermann

lib/nfs/FileReader: postpone the nfs_close_async() call

If an async opertion is in progress, nfs_close_async() will make libnfs crash because the RPC callback will dereference an object that was freed by nfs_close_async().
parent edd003b6
......@@ -150,6 +150,13 @@ public:
i->Cancel();
}
CT &Get(reference_type p) {
auto i = Find(p);
assert(i != list.end());
return *i;
}
};
#endif
......@@ -81,9 +81,22 @@ NfsConnection::CancellableCallback::Read(nfs_context *ctx, struct nfsfh *fh,
}
inline void
NfsConnection::CancellableCallback::CancelAndScheduleClose(struct nfsfh *fh)
{
assert(!open);
assert(close_fh == nullptr);
assert(fh != nullptr);
close_fh = fh;
Cancel();
}
inline void
NfsConnection::CancellableCallback::Callback(int err, void *data)
{
if (!IsCancelled()) {
assert(close_fh == nullptr);
NfsCallback &cb = Get();
connection.callbacks.Remove(*this);
......@@ -98,9 +111,12 @@ NfsConnection::CancellableCallback::Callback(int err, void *data)
/* a nfs_open_async() call was cancelled - to
avoid a memory leak, close the newly
allocated file handle immediately */
assert(close_fh == nullptr);
struct nfsfh *fh = (struct nfsfh *)data;
connection.Close(fh);
}
} else if (close_fh != nullptr)
connection.DeferClose(close_fh);
connection.callbacks.Remove(*this);
}
......@@ -135,6 +151,7 @@ NfsConnection::~NfsConnection()
assert(new_leases.empty());
assert(active_leases.empty());
assert(callbacks.IsEmpty());
assert(deferred_close.empty());
if (context != nullptr)
DestroyContext();
......@@ -225,6 +242,13 @@ NfsConnection::Close(struct nfsfh *fh)
}
void
NfsConnection::CancelAndClose(struct nfsfh *fh, NfsCallback &callback)
{
CancellableCallback &cancel = callbacks.Get(callback);
cancel.CancelAndScheduleClose(fh);
}
void
NfsConnection::DestroyContext()
{
assert(GetEventLoop().IsInside());
......@@ -237,6 +261,15 @@ NfsConnection::DestroyContext()
context = nullptr;
}
inline void
NfsConnection::DeferClose(struct nfsfh *fh)
{
assert(in_event);
assert(in_service);
deferred_close.push_front(fh);
}
void
NfsConnection::ScheduleSocket()
{
......@@ -257,6 +290,8 @@ NfsConnection::ScheduleSocket()
bool
NfsConnection::OnSocketReady(unsigned flags)
{
assert(deferred_close.empty());
bool closed = false;
const bool was_mounted = mount_finished;
......@@ -278,6 +313,12 @@ NfsConnection::OnSocketReady(unsigned flags)
assert(in_service);
in_service = false;
while (!deferred_close.empty()) {
nfs_close_async(context, deferred_close.front(),
DummyCallback, nullptr);
deferred_close.pop_front();
}
if (!was_mounted && mount_finished) {
if (postponed_mount_error.IsDefined()) {
DestroyContext();
......
......@@ -26,8 +26,11 @@
#include "event/DeferredMonitor.hxx"
#include "util/Error.hxx"
#include <boost/intrusive/list.hpp>
#include <string>
#include <list>
#include <forward_list>
struct nfs_context;
class NfsCallback;
......@@ -47,13 +50,19 @@ class NfsConnection : SocketMonitor, DeferredMonitor {
*/
const bool open;
/**
* The file handle scheduled to be closed as soon as
* the operation finishes.
*/
struct nfsfh *close_fh;
public:
explicit CancellableCallback(NfsCallback &_callback,
NfsConnection &_connection,
bool _open)
:CancellablePointer<NfsCallback>(_callback),
connection(_connection),
open(_open) {}
open(_open), close_fh(nullptr) {}
bool Open(nfs_context *context, const char *path, int flags,
Error &error);
......@@ -63,6 +72,12 @@ class NfsConnection : SocketMonitor, DeferredMonitor {
uint64_t offset, size_t size,
Error &error);
/**
* Cancel the operation and schedule a call to
* nfs_close_async() with the given file handle.
*/
void CancelAndScheduleClose(struct nfsfh *fh);
private:
static void Callback(int err, struct nfs_context *nfs,
void *data, void *private_data);
......@@ -79,6 +94,15 @@ class NfsConnection : SocketMonitor, DeferredMonitor {
typedef CancellableList<NfsCallback, CancellableCallback> CallbackList;
CallbackList callbacks;
/**
* A list of NFS file handles (struct nfsfh *) which shall be
* closed as soon as nfs_service() returns. If we close the
* file handle while in nfs_service(), libnfs may crash, and
* deferring this call to after nfs_service() avoids this
* problem.
*/
std::forward_list<struct nfsfh *> deferred_close;
Error postponed_mount_error;
/**
......@@ -135,6 +159,7 @@ public:
void Cancel(NfsCallback &callback);
void Close(struct nfsfh *fh);
void CancelAndClose(struct nfsfh *fh, NfsCallback &callback);
protected:
virtual void OnNfsConnectionError(Error &&error) = 0;
......@@ -145,6 +170,12 @@ private:
}
void DestroyContext();
/**
* Invoke nfs_close_async() after nfs_service() returns.
*/
void DeferClose(struct nfsfh *fh);
bool MountInternal(Error &error);
void BroadcastMountSuccess();
void BroadcastMountError(Error &&error);
......
......@@ -57,11 +57,18 @@ NfsFileReader::Close()
connection->RemoveLease(*this);
if (state > State::MOUNT && state != State::IDLE)
connection->Cancel(*this);
if (state > State::OPEN)
if (state == State::IDLE)
/* no async operation in progress: can close
immediately */
connection->Close(fh);
else if (state > State::OPEN)
/* one async operation in progress: cancel it and
defer the nfs_close_async() call */
connection->CancelAndClose(fh, *this);
else if (state > State::MOUNT)
/* we don't have a file handle yet - just cancel the
async operation */
connection->Cancel(*this);
state = State::INITIAL;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment