From 2f7eb9b5460036d97b4ef370637f721adf6b6c96 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Sun, 13 Dec 2009 19:28:33 +0000 Subject: [PATCH] Fixed a stupid race condition between IORequest finishing and IORequest::Wait(). Wait() immediately returned when IsFinished() returned true, but this is the case as soon as the last IOOperation has finished. The I/O scheduler is not done with the request at this point, though, since it will still be sitting in at least one of three doubly linked lists. Since the usual procedure to issue synchronous I/O requests is to create an IORequest on the stack, pass it to the I/O scheduler, and Wait() on it, Wait() returning early might cause the IORequest object to be destroyed while it is still in use, leading to invalid memory access in the I/O scheduler, corruption of its list structures, as well as later corruption of the issuing thread's stack. Related tickets: * #4431: The request issuing thread returned and already deleted the area the request was writing to before NotifyFinished() was called. * #3048, #4883: Caused by the on stack IORequest being overwritten with other data while being handled by the I/O scheduler thread. * #4517: Hard to say, but I've seen a such a problem too, after a thread scheduling related change. An explanation would be a list structure corruption in the I/O scheduler causing an infinite loop with disabled interrupts. * #2845, #3428, #3429: The block notifier/writer is I/O heavy and as such quite likely to run into the stack corruption issue. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@34655 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/system/kernel/device_manager/IORequest.cpp | 4 +++- src/system/kernel/device_manager/IORequest.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/system/kernel/device_manager/IORequest.cpp b/src/system/kernel/device_manager/IORequest.cpp index 2999ad58f8..a724fc36df 100644 --- a/src/system/kernel/device_manager/IORequest.cpp +++ b/src/system/kernel/device_manager/IORequest.cpp @@ -692,6 +692,7 @@ IOOperation::Dump() const IORequest::IORequest() : + fIsNotified(false), fFinishedCallback(NULL), fFinishedCookie(NULL), fIterationCallback(NULL), @@ -869,7 +870,7 @@ IORequest::Wait(uint32 flags, bigtime_t timeout) { MutexLocker locker(fLock); - if (IsFinished()) + if (IsFinished() && fIsNotified) return Status(); ConditionVariableEntry entry; @@ -932,6 +933,7 @@ IORequest::NotifyFinished() bool deleteRequest = (fFlags & B_DELETE_IO_REQUEST) != 0; // unblock waiters + fIsNotified = true; fFinishedCondition.NotifyAll(); locker.Unlock(); diff --git a/src/system/kernel/device_manager/IORequest.h b/src/system/kernel/device_manager/IORequest.h index bb6f90a42d..8fdbad7eeb 100644 --- a/src/system/kernel/device_manager/IORequest.h +++ b/src/system/kernel/device_manager/IORequest.h @@ -334,6 +334,7 @@ private: bool fIsWrite; bool fPartialTransfer; bool fSuppressChildNotifications; + bool fIsNotified; io_request_finished_callback fFinishedCallback; void* fFinishedCookie;