From 5de0f22264f580c427af739185c6bab8e8d881be Mon Sep 17 00:00:00 2001 From: psychon Date: Tue, 27 Jan 2009 18:37:23 +0000 Subject: Fix the poll socketengine to actually work. The old implementation tried to use holes for unused entries in the array for poll() and the OS replied with POLLNVAL to which we replied with 100% CPU. Improvements: - It does compile! - It works git-svn-id: http://svn.inspircd.org/repository/trunk/inspircd@11008 e03df62e-2008-0410-955e-edbf42e46eb7 --- include/socketengines/socketengine_poll.h | 4 ++ src/socketengines/socketengine_poll.cpp | 109 ++++++++++++++++++++++-------- 2 files changed, 83 insertions(+), 30 deletions(-) diff --git a/include/socketengines/socketengine_poll.h b/include/socketengines/socketengine_poll.h index f40cecd2c..1a85cff5e 100644 --- a/include/socketengines/socketengine_poll.h +++ b/include/socketengines/socketengine_poll.h @@ -43,6 +43,9 @@ private: /** These are used by poll() to hold socket events */ struct pollfd *events; + /** This map maps fds to an index in the events array. + */ + std::map fd_mappings; public: /** Create a new PollEngine * @param Instance The creator of this object @@ -52,6 +55,7 @@ public: */ virtual ~PollEngine(); virtual bool AddFd(EventHandler* eh); + virtual EventHandler* GetRef(int fd); virtual int GetMaxFds(); virtual int GetRemainingFds(); virtual bool DelFd(EventHandler* eh, bool force = false); diff --git a/src/socketengines/socketengine_poll.cpp b/src/socketengines/socketengine_poll.cpp index c05ed25fa..242a4d96f 100644 --- a/src/socketengines/socketengine_poll.cpp +++ b/src/socketengines/socketengine_poll.cpp @@ -54,31 +54,49 @@ bool PollEngine::AddFd(EventHandler* eh) return false; } - if (ref[fd]) + if (fd_mappings.find(fd) != fd_mappings.end()) { ServerInstance->Logs->Log("SOCKET",DEBUG,"Attempt to add duplicate fd: %d", fd); return false; } - ref[fd] = eh; - events[fd].fd = fd; + unsigned int index = CurrentSetSize; + + fd_mappings[fd] = index; + ref[index] = eh; + events[index].fd = fd; if (eh->Readable()) { - events[fd].events = POLLIN; + events[index].events = POLLIN; } else { - events[fd].events = POLLOUT; + events[index].events = POLLOUT; } - ServerInstance->Logs->Log("SOCKET", DEBUG,"New file descriptor: %d (%d)", fd, events[fd].events); + ServerInstance->Logs->Log("SOCKET", DEBUG,"New file descriptor: %d (%d; index %d)", fd, events[fd].events, index); CurrentSetSize++; return true; } +EventHandler* PollEngine::GetRef(int fd) +{ + std::map::iterator it = fd_mappings.find(fd); + if (it == fd_mappings.end()) + return NULL; + return ref[it->second]; +} + void PollEngine::WantWrite(EventHandler* eh) { - events[eh->GetFd()].events = POLLIN | POLLOUT; + std::map::iterator it = fd_mappings.find(eh->GetFd()); + if (it == fd_mappings.end()) + { + ServerInstance->Logs->Log("SOCKET",DEBUG,"WantWrite() on unknown fd: %d", eh->GetFd()); + return; + } + + events[it->second].events = POLLIN | POLLOUT; } bool PollEngine::DelFd(EventHandler* eh, bool force) @@ -90,13 +108,42 @@ bool PollEngine::DelFd(EventHandler* eh, bool force) return false; } - events[fd].fd = -1; - events[fd].events = 0; + std::map::iterator it = fd_mappings.find(fd); + if (it == fd_mappings.end()) + { + ServerInstance->Logs->Log("SOCKET",DEBUG,"DelFd() on unknown fd: %d", fd); + return false; + } + + unsigned int index = it->second; + unsigned int last_index = CurrentSetSize - 1; + int last_fd = events[last_index].fd; + + if (index != last_index) + { + // We need to move the last fd we got into this gap (gaps are evil!) + + // So update the mapping for the last fd to its new position + fd_mappings[last_fd] = index; + + // move last_fd from last_index into index + events[index].fd = last_fd; + events[index].events = events[last_index].events; + + ref[index] = ref[last_index]; + } + + // Now remove all data for the last fd we got into out list. + // Above code made sure this always is right + fd_mappings.erase(it); + events[last_index].fd = 0; + events[last_index].events = 0; + ref[last_index] = NULL; CurrentSetSize--; - ref[fd] = NULL; - ServerInstance->Logs->Log("SOCKET", DEBUG, "Remove file descriptor: %d", fd); + ServerInstance->Logs->Log("SOCKET", DEBUG, "Remove file descriptor: %d (index: %d) " + "(Filled gap with: %d (index: %d))", fd, index, last_fd, last_index); return true; } @@ -116,7 +163,7 @@ int PollEngine::GetMaxFds() { MAX_DESCRIPTORS = 0; ServerInstance->Logs->Log("SOCKET", DEFAULT, "ERROR: Can't determine maximum number of open sockets: %s", strerror(errno)); - printf("ERROR: Can't determine maximum number of open sockets: %s\n", strerror(errno))); + printf("ERROR: Can't determine maximum number of open sockets: %s\n", strerror(errno)); ServerInstance->Exit(EXIT_STATUS_SOCKETENGINE); } return 0; @@ -144,51 +191,53 @@ int PollEngine::GetRemainingFds() int PollEngine::DispatchEvents() { - int i = poll(events, GetMaxFds() - 1, 1000); - int fd = 0; + int i = poll(events, CurrentSetSize, 1000); + int index; socklen_t codesize = sizeof(int); int errcode; int processed = 0; if (i > 0) { - for (fd = 0; fd < GetMaxFds() - 1 && processed != i; fd++) + for (index = 0; index < CurrentSetSize && processed != i; index++) { - if (events[fd].revents) + if (events[index].revents) processed++; - if (events[fd].revents & POLLHUP) + if (events[index].revents & POLLHUP) { - if (ref[fd]) - ref[fd]->HandleEvent(EVENT_ERROR, 0); + if (ref[index]) + ref[index]->HandleEvent(EVENT_ERROR, 0); continue; } - if (events[fd].revents & POLLERR) + if (events[index].revents & POLLERR) { + // Get fd + int fd = events[index].fd; + // Get error number if (getsockopt(fd, SOL_SOCKET, SO_ERROR, &errcode, &codesize) < 0) errcode = errno; - if (ref[fd]) - ref[fd]->HandleEvent(EVENT_ERROR, errcode); + if (ref[index]) + ref[index]->HandleEvent(EVENT_ERROR, errcode); continue; } - if (events[fd].revents & POLLOUT) + if (events[index].revents & POLLOUT) { // Switch to wanting read again // event handlers have to request to write again if they need it - events[fd].events = POLLIN; - + events[index].events = POLLIN; - if (ref[fd]) - ref[fd]->HandleEvent(EVENT_WRITE); + if (ref[index]) + ref[index]->HandleEvent(EVENT_WRITE); } - if (events[fd].revents & POLLIN) + if (events[index].revents & POLLIN) { - if (ref[fd]) - ref[fd]->HandleEvent(EVENT_READ); + if (ref[index]) + ref[index]->HandleEvent(EVENT_READ); } } } -- cgit v1.2.3