1
0
Fork 0
mirror of https://github.com/juce-framework/JUCE.git synced 2026-01-09 23:34:20 +00:00

Network: Fix potential deadlock in macOS WebInputStream

Regarding didComplete():

When a WebInputStream is destroyed, the thread of execution will wait in
SharedSession::removeTask() until the task is no longer present in the
list of all active tasks. If multiple threads are all waiting in
removeTask(), then all of those threads should wake when the set of
active tasks changes. Waking only a single thread may result in
deadlocks, as that thread's task may not have completed successfully.
Then, the thread that woke up will be forced to sleep again and may not
get another chance to wake.

Regarding didBecomeInvalid():

Normally, didBecomeInvalid() will only be called after the
SharedSession's destructor. If the destructor is running, we may assume
that no other thread can access the SharedSession, so using notify_one()
in didBecomeInvalid() should be sufficient to wake up the destructor's
thread. However, there's a chance that the NSURLSession may be
invalidated unexpectedly (i.e. before the SharedSession's destructor
runs), in which case there may still be threads waiting in removeTask().
In this scenario we need to notify_all() so that all waiting threads are
able to wake and make progress.
This commit is contained in:
reuk 2025-06-30 18:55:47 +01:00
parent a2b2813b93
commit 21b0cd4663
No known key found for this signature in database

View file

@ -140,15 +140,18 @@ public:
~SharedSession()
{
std::unique_lock lock { mutex };
[session.get() finishTasksAndInvalidate];
condvar.wait (lock, [&] { return state == State::stopped; });
if (session != nullptr)
[session.get() finishTasksAndInvalidate];
condvar.wait (lock, [&] { return session == nullptr; });
}
NSUniquePtr<NSURLSessionTask> addTask (NSURLRequest* request, SessionListener* listener)
{
std::unique_lock lock { mutex };
if (state != State::running)
if (session == nullptr)
return nullptr;
NSUniquePtr<NSURLSessionTask> task { [[session.get() dataTaskWithRequest: request] retain] };
@ -170,28 +173,28 @@ private:
DBG (nsStringToJuce ([error description]));
#endif
const auto toNotify = [&]
const auto toNotify = std::invoke ([&]
{
const std::scoped_lock lock { mutex };
state = State::stopRequested;
session.reset();
// Take a copy of listenerForTask so that we don't need to hold the lock while
// iterating through the remaining listeners.
return listenerForTask;
}();
return std::exchange (listenerForTask, {});
});
for (const auto& pair : toNotify)
pair.second->didComplete (error);
const std::scoped_lock lock { mutex };
listenerForTask.clear();
state = State::stopped;
// Important: we keep the lock held while calling condvar.notify_one().
// Important: we keep the lock held while calling condvar.notify_all().
// If we don't, then it's possible that when the destructor runs, it will wake
// before we notify the condvar on this thread, allowing the destructor to continue
// and destroying the condition variable. When didBecomeInvalid resumes, the condition
// variable will have been destroyed.
condvar.notify_one();
// Use notify_all() rather than notify_one() so that all threads waiting
// in removeTask() can make progress in the case that the session is
// invalidated unexpectedly.
const std::scoped_lock lock { mutex };
condvar.notify_all();
}
void didComplete (NSURLSessionTask* task, [[maybe_unused]] NSError* error)
@ -213,7 +216,7 @@ private:
listenerForTask.erase ([task taskIdentifier]);
}
condvar.notify_one();
condvar.notify_all();
}
void didReceiveResponse (NSURLSessionTask* task,
@ -324,13 +327,6 @@ private:
}
};
enum class State
{
running,
stopRequested,
stopped,
};
std::mutex mutex;
std::condition_variable condvar;
@ -343,8 +339,6 @@ private:
};
std::map<NSUInteger, SessionListener*> listenerForTask;
State state = State::running;
};
class TaskToken