changeset 363:3908306107d4

Socket: unbreak poll on Windows and fix kqueue
author David Demelier <markand@malikania.fr>
date Tue, 28 Apr 2015 14:40:17 +0200
parents ff5b46474895
children d88c3644ebc8
files C++/modules/Socket/SocketListener.cpp C++/modules/Socket/SocketListener.h
diffstat 2 files changed, 67 insertions(+), 55 deletions(-) [+]
line wrap: on
line diff
--- a/C++/modules/Socket/SocketListener.cpp	Tue Apr 28 11:50:44 2015 +0200
+++ b/C++/modules/Socket/SocketListener.cpp	Tue Apr 28 14:40:17 2015 +0200
@@ -30,20 +30,19 @@
 
 namespace backend {
 
-void Select::set(Socket s, int direction)
+void Select::set(Socket s, int flags)
 {
 	if (m_table.count(s.handle()) > 0) {
-		m_table.at(s.handle()).second |= direction;
+		m_table.at(s.handle()).second |= flags;
 	} else {
-		m_table.insert({s.handle(), {s, direction}});
+		m_table.emplace(s.handle(), std::make_pair(s, flags));
 	}
-
 }
 
-void Select::unset(Socket s, int direction)
+void Select::unset(Socket s, int flags)
 {
 	if (m_table.count(s.handle()) != 0) {
-		m_table.at(s.handle()).second &= ~(direction);
+		m_table.at(s.handle()).second &= ~(flags);
 
 		// If no read, no write is requested, remove it
 		if (m_table.at(s.handle()).second == 0) {
@@ -64,7 +63,7 @@
 
 SocketStatus Select::wait(int ms)
 {
-	auto result = waitMultiple(ms);
+	std::vector<SocketStatus> result = waitMultiple(ms);
 
 	if (result.size() == 0) {
 		throw SocketError(SocketError::System, "select", "No socket found");
@@ -115,10 +114,10 @@
 
 	for (auto &c : m_table) {
 		if (FD_ISSET(c.first, &readset)) {
-			sockets.push_back({ c.second.first, SocketListener::Read });
+			sockets.push_back(SocketStatus{c.second.first, SocketListener::Read});
 		}
 		if (FD_ISSET(c.first, &writeset)) {
-			sockets.push_back({ c.second.first, SocketListener::Write });
+			sockets.push_back(SocketStatus{c.second.first, SocketListener::Write});
 		}
 	}
 
@@ -135,23 +134,23 @@
 #  define poll WSAPoll
 #endif
 
-short Poll::topoll(int direction) const noexcept
+short Poll::topoll(int flags) const noexcept
 {
 	short result(0);
 
-	if (direction & SocketListener::Read) {
+	if (flags & SocketListener::Read) {
 		result |= POLLIN;
 	}
-	if (direction & SocketListener::Write) {
+	if (flags & SocketListener::Write) {
 		result |= POLLOUT;
 	}
 
 	return result;
 }
 
-int Poll::todirection(short event) const noexcept
+int Poll::toflags(short &event) const noexcept
 {
-	int direction = 0;
+	int flags = 0;
 
 	/*
 	 * Poll implementations mark the socket differently regarding
@@ -161,33 +160,38 @@
 	 * return 0 so we mark the socket as readable.
 	 */
 	if ((event & POLLIN) || (event & POLLHUP)) {
-		direction |= SocketListener::Read;
+		flags |= SocketListener::Read;
 	}
 	if (event & POLLOUT) {
-		direction |= SocketListener::Write;
+		flags |= SocketListener::Write;
 	}
 
-	return direction;
+	// Reset event for safety
+	event = 0;
+
+	return flags;
 }
 
-void Poll::set(Socket s, int direction)
+void Poll::set(Socket s, int flags)
 {
-	auto it = std::find_if(m_fds.begin(), m_fds.end(), [&] (const auto &pfd) { return pfd.fd == s.handle(); });
+	auto it = std::find_if(m_fds.begin(), m_fds.end(), [&] (const auto &pfd) {
+		return pfd.fd == s.handle();
+	});
 
-	// If found, add the new direction, otherwise add a new socket
+	// If found, add the new flags, otherwise add a new socket
 	if (it != m_fds.end()) {
-		it->events |= topoll(direction);
+		it->events |= topoll(flags);
 	} else {
-		m_lookup.insert({s.handle(), s});
-		m_fds.push_back({ s.handle(), topoll(direction), 0 });
+		m_lookup.emplace(s.handle(), s);
+		m_fds.push_back({ s.handle(), topoll(flags), 0 });
 	}
 }
 
-void Poll::unset(Socket s, int direction)
+void Poll::unset(Socket s, int flags)
 {
 	for (auto i = m_fds.begin(); i != m_fds.end();) {
 		if (i->fd == s.handle()) {
-			i->events &= ~(topoll(direction));
+			i->events &= ~(topoll(flags));
 
 			if (i->events == 0) {
 				m_lookup.erase(i->fd);
@@ -229,7 +233,7 @@
 
 	for (auto &fd : m_fds) {
 		if (fd.revents != 0) {
-			return { m_lookup.at(fd.fd), todirection(fd.revents) };
+			return SocketStatus{m_lookup.at(fd.fd), toflags(fd.revents)};
 		}
 	}
 
@@ -249,7 +253,7 @@
 	std::vector<SocketStatus> sockets;
 	for (auto &fd : m_fds) {
 		if (fd.revents != 0) {
-			sockets.push_back({ m_lookup.at(fd.fd), todirection(fd.revents) });
+			sockets.push_back(SocketStatus{m_lookup.at(fd.fd), toflags(fd.revents)});
 		}
 	}
 
@@ -278,48 +282,52 @@
 	});
 }
 
-std::vector<struct kevent>::iterator Kqueue::find(const Socket &s)
+std::vector<kevent>::iterator Kqueue::find(const Socket &s) const
 {
-	return std::find_if(m_list.begin(), m_list.end(), [&] (struct kevent &kv) -> bool {
+	return std::find_if(m_list.begin(), m_list.end(), [&] (const struct kevent &kv) -> bool {
 		return static_cast<Socket::Handle>(kv.ident) == s.handle();
 	});
 }
 
-void Kqueue::set(Socket &s, int direction)
+void Kqueue::set(Socket &s, int flags)
 {
-	struct kevent ev;
+	auto it = find(s);
+	int filter = 0;
 
-	if (direction == SocketListener::Read) {
-		EV_SET(&ev, s.handle(), EVFILT_READ, EV_ADD | EV_ENABLE, 0, 0, &s);
-	} else if (direction == SocketListener::Write) {
-		EV_SET(&ev, s.handle(), EVFILT_WRITE, EV_ADD | EV_ENABLE, 0, 0, &s);
+	if (flags & SocketListener::Read) {
+		filter |= EVFILT_READ;
+	}
+	if (flags & SocketListener::Write) {
+		filter |= EVFILT_WRITE;
 	}
 
-	auto it = find(s);
+	if (it == m_list.end()) {
+		struct kevent ev;
 
-	if (it == m_list.end()) {
+		EV_SET(&ev, s.handle(), filter, EV_ADD | EV_ENABLE, 0, 0, &s);
+
 		m_list.push_back(ev);
 	} else {
-		*it = ev;
+		it->filter |= filter;
 	}
 
 	m_result.resize(m_list.size());
 }
 
-void Kqueue::unset(Socket s, int direction)
+void Kqueue::unset(Socket s, int flags)
 {
 	auto it = find(s);
 
 	if (it != m_list.end()) {
-		if (direction & SocketListener::Read) {
+		if (flags & SocketListener::Read) {
 			it->filter &= ~(EVFILT_READ);
 		}
-		if (direction & SocketListener::Write) {
+		if (flags & SocketListener::Write) {
 			it->filter &= ~(EVFILT_WRITE);
 		}
 
 		/* complete removal */
-		if ((it->filter & ~(direction)) == 0) {
+		if (it->filter == 0) {
 			m_list.erase(it);
 		}
 	}
--- a/C++/modules/Socket/SocketListener.h	Tue Apr 28 11:50:44 2015 +0200
+++ b/C++/modules/Socket/SocketListener.h	Tue Apr 28 14:40:17 2015 +0200
@@ -91,7 +91,7 @@
 
 #include "Socket.h"
 
-#if defined(SOCKET_HAVE_POLL)
+#if defined(SOCKET_HAVE_POLL) && !defined(_WIN32)
 #  include <poll.h>
 #endif
 
@@ -121,8 +121,8 @@
 	std::map<Socket::Handle, std::pair<Socket, int>> m_table;
 
 public:
-	void set(Socket s, int direction);
-	void unset(Socket s, int direction);
+	void set(Socket s, int flags);
+	void unset(Socket s, int flags);
 	void remove(Socket sc);
 	void clear();
 	SocketStatus wait(int ms);
@@ -143,12 +143,12 @@
 	std::vector<pollfd> m_fds;
 	std::map<Socket::Handle, Socket> m_lookup;
 
-	short topoll(int direction) const noexcept;
-	int todirection(short event) const noexcept;
+	short topoll(int flags) const noexcept;
+	int toflags(short &event) const noexcept;
 
 public:
-	void set(Socket s, int direction);
-	void unset(Socket s, int direction);
+	void set(Socket s, int flags);
+	void unset(Socket s, int flags);
 	void remove(Socket sc);
 	void clear();
 	SocketStatus wait(int ms);
@@ -183,15 +183,15 @@
 	Kqueue(const Kqueue &) = delete;
 	Kqueue &operator=(const Kqueue &) = delete;
 
-	std::vector<struct kevent>::iterator find(Socket &s);
+	std::vector<struct kevent>::iterator find(Socket &s) const;
 
 public:
 	Kqueue();
 	Kqueue(Kqueue &&) = default;
 	Kqueue &operator=(Kqueue &&) = default;
 
-	void set(Socket &s, int direction);
-	void unset(Socket &s, int direction);
+	void set(Socket &s, int flags);
+	void unset(Socket &s, int flags);
 	void remove(Socket &sc);
 	void clear();
 	SocketStatus wait(int ms);
@@ -211,6 +211,10 @@
  * This class is implemented using a bridge pattern to allow different uses
  * of listener implementation.
  *
+ * You should not reinstanciate a new SocketListener at each iteartion of your
+ * main loop as it can be extremely costly. Instead use the same listener that
+ * you can safely modify on the fly.
+ *
  * Currently, poll, select and kqueue are available.
  */
 template <typename Backend = SOCKET_DEFAULT_BACKEND>
@@ -320,11 +324,11 @@
 	}
 
 	/**
-	 * Unset a socket from the listener, only the direction is removed
-	 * unless the two directions are requested.
+	 * Unset a socket from the listener, only the flags is removed
+	 * unless the two flagss are requested.
 	 *
 	 * For example, if you added a socket for both reading and writing,
-	 * unsetting the write direction will keep the socket for reading.
+	 * unsetting the write flags will keep the socket for reading.
 	 *
 	 * @param sc the socket
 	 * @param flags the flags (may be OR'ed)