You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by as...@apache.org on 2008/07/28 18:45:26 UTC
svn commit: r680395 - in /incubator/qpid/trunk/qpid/cpp/src/qpid/sys:
Dispatcher.cpp Poller.h epoll/EpollPoller.cpp
Author: astitcher
Date: Mon Jul 28 09:45:26 2008
New Revision: 680395
URL: http://svn.apache.org/viewvc?rev=680395&view=rev
Log:
Refactor of EpollPoller to make PollerHandler lifecycle easier
Modified:
incubator/qpid/trunk/qpid/cpp/src/qpid/sys/Dispatcher.cpp
incubator/qpid/trunk/qpid/cpp/src/qpid/sys/Poller.h
incubator/qpid/trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp
Modified: incubator/qpid/trunk/qpid/cpp/src/qpid/sys/Dispatcher.cpp
URL: http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/cpp/src/qpid/sys/Dispatcher.cpp?rev=680395&r1=680394&r2=680395&view=diff
==============================================================================
--- incubator/qpid/trunk/qpid/cpp/src/qpid/sys/Dispatcher.cpp (original)
+++ incubator/qpid/trunk/qpid/cpp/src/qpid/sys/Dispatcher.cpp Mon Jul 28 09:45:26 2008
@@ -339,7 +339,7 @@
}
}
// If we're not then do it right away
- deferDelete();
+ delete this;
}
void DispatchHandle::processEvent(Poller::EventType type) {
@@ -433,7 +433,7 @@
break;
}
}
- deferDelete();
+ delete this;
}
}}
Modified: incubator/qpid/trunk/qpid/cpp/src/qpid/sys/Poller.h
URL: http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/cpp/src/qpid/sys/Poller.h?rev=680395&r1=680394&r2=680395&view=diff
==============================================================================
--- incubator/qpid/trunk/qpid/cpp/src/qpid/sys/Poller.h (original)
+++ incubator/qpid/trunk/qpid/cpp/src/qpid/sys/Poller.h Mon Jul 28 09:45:26 2008
@@ -96,17 +96,11 @@
public:
PollerHandle(const IOHandle& h);
-
- // Usual way to delete (will defer deletion until we
- // can't be returned from a Poller::wait any more)
- void deferDelete();
-
- // Class clients shouldn't ever use this
virtual ~PollerHandle();
};
inline void Poller::Event::process() {
- handle->processEvent(type);
+ handle->processEvent(type);
}
}}
Modified: incubator/qpid/trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp
URL: http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp?rev=680395&r1=680394&r2=680395&view=diff
==============================================================================
--- incubator/qpid/trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp (original)
+++ incubator/qpid/trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp Mon Jul 28 09:45:26 2008
@@ -37,11 +37,11 @@
namespace sys {
// Deletion manager to handle deferring deletion of PollerHandles to when they definitely aren't being used
-DeletionManager<PollerHandle> PollerHandleDeletionManager;
+DeletionManager<PollerHandlePrivate> PollerHandleDeletionManager;
// Instantiate (and define) class static for DeletionManager
template <>
-DeletionManager<PollerHandle>::AllThreadsStatuses DeletionManager<PollerHandle>::allThreadsStatuses(0);
+DeletionManager<PollerHandlePrivate>::AllThreadsStatuses DeletionManager<PollerHandlePrivate>::allThreadsStatuses(0);
class PollerHandlePrivate {
friend class Poller;
@@ -52,20 +52,23 @@
MONITORED,
INACTIVE,
HUNGUP,
- MONITORED_HUNGUP
+ MONITORED_HUNGUP,
+ DELETED
};
int fd;
::__uint32_t events;
+ PollerHandle* pollerHandle;
FDStat stat;
Mutex lock;
- PollerHandlePrivate(int f) :
+ PollerHandlePrivate(int f, PollerHandle* p) :
fd(f),
events(0),
+ pollerHandle(p),
stat(ABSENT) {
}
-
+
bool isActive() const {
return stat == MONITORED || stat == MONITORED_HUNGUP;
}
@@ -98,18 +101,26 @@
assert(stat == MONITORED);
stat = HUNGUP;
}
+
+ bool isDeleted() const {
+ return stat == DELETED;
+ }
+
+ void setDeleted() {
+ stat = DELETED;
+ }
};
PollerHandle::PollerHandle(const IOHandle& h) :
- impl(new PollerHandlePrivate(toFd(h.impl)))
+ impl(new PollerHandlePrivate(toFd(h.impl), this))
{}
PollerHandle::~PollerHandle() {
- delete impl;
-}
-
-void PollerHandle::deferDelete() {
- PollerHandleDeletionManager.markForDeletion(this);
+ ScopedLock<Mutex> l(impl->lock);
+ if (impl->isActive()) {
+ impl->setDeleted();
+ }
+ PollerHandleDeletionManager.markForDeletion(impl);
}
/**
@@ -201,7 +212,7 @@
op = EPOLL_CTL_MOD;
epe.events = eh.events | PollerPrivate::directionToEpollEvent(dir);
}
- epe.data.ptr = &handle;
+ epe.data.ptr = &eh;
QPID_POSIX_CHECK(::epoll_ctl(impl->epollFd, op, eh.fd, &epe));
@@ -231,7 +242,7 @@
::epoll_event epe;
epe.events = PollerPrivate::directionToEpollEvent(dir) | ::EPOLLONESHOT;
- epe.data.ptr = &handle;
+ epe.data.ptr = &eh;
QPID_POSIX_CHECK(::epoll_ctl(impl->epollFd, EPOLL_CTL_MOD, eh.fd, &epe));
@@ -247,7 +258,7 @@
::epoll_event epe;
epe.events = eh.events;
- epe.data.ptr = &handle;
+ epe.data.ptr = &eh;
QPID_POSIX_CHECK(::epoll_ctl(impl->epollFd, EPOLL_CTL_MOD, eh.fd, &epe));
@@ -284,6 +295,7 @@
int rc = ::epoll_wait(impl->epollFd, &epe, 1, timeoutMs);
if (impl->isShutdown) {
+ PollerHandleDeletionManager.markAllUnusedInThisThread();
return Event(0, SHUTDOWN);
}
@@ -291,13 +303,14 @@
QPID_POSIX_CHECK(rc);
} else if (rc > 0) {
assert(rc == 1);
- PollerHandle* handle = static_cast<PollerHandle*>(epe.data.ptr);
- PollerHandlePrivate& eh = *handle->impl;
+ PollerHandlePrivate& eh = *static_cast<PollerHandlePrivate*>(epe.data.ptr);
ScopedLock<Mutex> l(eh.lock);
// the handle could have gone inactive since we left the epoll_wait
if (eh.isActive()) {
+ PollerHandle* handle = eh.pollerHandle;
+
// If the connection has been hungup we could still be readable
// (just not writable), allow us to readable until we get here again
if (epe.events & ::EPOLLHUP) {
@@ -309,6 +322,14 @@
eh.setInactive();
}
return Event(handle, PollerPrivate::epollToDirection(epe.events));
+ } else if (eh.isDeleted()) {
+ // The handle has been deleted whilst still active and so must be removed
+ // from the poller
+ int rc = ::epoll_ctl(impl->epollFd, EPOLL_CTL_DEL, eh.fd, 0);
+ // Ignore EBADF since it's quite likely that we could race with closing the fd
+ if (rc == -1 && errno != EBADF) {
+ QPID_POSIX_CHECK(rc);
+ }
}
}
// We only get here if one of the following:
@@ -323,6 +344,7 @@
// with a timeout as we don't know how long we've waited so far and so we can't
// continue the wait.
if (rc == 0 || timeoutMs != -1) {
+ PollerHandleDeletionManager.markAllUnusedInThisThread();
return Event(0, TIMEOUT);
}
} while (true);