For learning purposes I am trying to develop an event-based library/wrapper to communicate with many clients by using epoll
.
The wrapper was designed with the following criteria in mind:
- An Object-Oriented design
- Very error robust - Handling of most socket exceptions
- Lightweight and compact
This code is based on what I've learned by studying multiple other projects and by reading the official docs. I like to know if my wrapper is using epoll as it is supposed to be used and if I can fine-tune my lib any further.
To simulate incoming client connections I used putty - for example to listen for incoming connections on port 1337 simply do: putty 127.0.0.1 -P 1337
Once the connection is established all the clients can send data to the server.
I am by no means an expert with sockets, so feel free to be very nit-picky. That helps the most to improve the code :)
This is how I realized my wrapper:
Socket.cpp
#include <iostream>
#include <fcntl.h>
#include "Socket.h"
Socket::Socket(uint16_t port) {
this->listeningPort = port;
}
bool Socket::initialize() {
// Setup address structs
address.sin_family = AF_INET;
address.sin_addr.s_addr = INADDR_ANY;
address.sin_port = htons(listeningPort);
// Check if port was set
if (listeningPort == 0) {
raiseSocketExceptionEvent("Invalid port choice.");
return false;
}
// Create socket
if (0 == (masterSocket = socket(AF_INET, SOCK_STREAM, 0))) {
raiseSocketExceptionEvent("Failed to create socket.");
return false;
}
// Set the listening socket to be non-blocking
setNonBlocking(masterSocket);
// Specify socket options
if (setsockopt(masterSocket, SOL_SOCKET, SO_REUSEADDR, &this->socketOptions, sizeof(socketOptions)) == -1) {
raiseSocketExceptionEvent("Failed to set socket options.");
return false;
}
// Bind socket to port
if (bind(masterSocket, (struct sockaddr *) &address, sizeof(address)) != 0) {
raiseSocketExceptionEvent("Failed to bind socket.");
return false;
}
return true;
}
bool Socket::start() {
if (listen(masterSocket, SOMAXCONN) < 0) {
raiseSocketExceptionEvent("Failed to start listening.");
return false;
}
if ((epollFd = epoll_create1(0)) < 0) {
raiseSocketExceptionEvent("Failed to create epoll instance");
return false;
}
// Configure listening socket for all epoll operations
ev.data.fd = masterSocket;
ev.events = EPOLLIN | EPOLLPRI | EPOLLERR | EPOLLHUP | EPOLLET;
if (epoll_ctl(epollFd, EPOLL_CTL_ADD, masterSocket, &ev) < 0) {
raiseSocketExceptionEvent("Failed to configure epoll file descriptor");
return false;
}
while (running) {
// Get number of file descriptors that are ready for I/O
int nfds = epoll_wait(epollFd, events, MAX_EVENTS, timeout);
if (nfds == -1) {
raiseSocketExceptionEvent("epoll_wait() failed to wait for events.");
close(nfds);
close(masterSocket);
return false;
}
// Iterate all events
for (int i = 0; i < nfds; ++i) {
if (events[i].data.fd == masterSocket)
handleListeningFileDescriptorActivity();
else
handleClientFileDescriptorActivity(i);
}
}
return true;
}
// Event methods
void Socket::removeEventListener(ISocketEventListener *listener) {
for (int i = 0; i < eventListeners.size(); i++) {
if (eventListeners[i] == listener) {
eventListeners.erase(eventListeners.begin() + i);
break;
}
}
}
void Socket::addEventListener(ISocketEventListener *listener) {
eventListeners.push_back(listener);
}
// Socket Events
void Socket::raiseClientDisconnectedEvent(int socket) const {
for (int i = 0; i < eventListeners.size(); i++) {
eventListeners[i]->onDisconnect(SocketConnectEventArgs(socket));
}
}
void Socket::raiseClientConnectedEvent(int socket) const {
for (int i = 0; i < eventListeners.size(); i++) {
eventListeners[i]->onConnect(SocketConnectEventArgs(socket));
}
}
void Socket::raiseDataSendEvent(const std::vector<char> &data, int fd) const {
for (int i = 0; i < eventListeners.size(); i++) {
eventListeners[i]->onDataSend(SocketDataEventArgs(data, fd));
}
}
void Socket::raiseDataReceivedEvent(const std::vector<char> &data, int fd) const {
for (int i = 0; i < eventListeners.size(); i++) {
eventListeners[i]->onDataReceive(SocketDataEventArgs(data, fd));
}
}
void Socket::raiseSocketExceptionEvent(std::string message) const {
for (int i = 0; i < eventListeners.size(); i++) {
eventListeners[i]->onSocketException(SocketExceptionEventArgs(message));
}
}
// Private methods
void Socket::setNonBlocking(int socket) {
int options;
if ((options = fcntl(socket, F_GETFL)) < 0) {
raiseSocketExceptionEvent("F_GETFL failed.");
return;
}
options = options | O_NONBLOCK;
if (fcntl(socket, F_SETFL, options) < 0) {
raiseSocketExceptionEvent("F_SETFL failed.");
return;
}
}
void Socket::handleListeningFileDescriptorActivity() {
// accept client connection
socklen_t addressLen = sizeof(address);
int fd = accept(masterSocket, (struct sockaddr *) &address, &addressLen);
if (fd < 0) {
raiseSocketExceptionEvent("Invalid file descriptor from accepted connection ");
return;
}
// Set fd to be non blocking
setNonBlocking(fd);
// Monitor read operations, set edge-triggered
ev.events = EPOLLIN | EPOLLOUT | EPOLLET;
ev.data.fd = fd;
// Add fd to epoll data structure
if (epoll_ctl(epollFd, EPOLL_CTL_ADD, fd, &ev) < 0) {
raiseSocketExceptionEvent("Failed to add connected socket fd to epoll");
return;
}
// Notify that a new client connected
raiseClientConnectedEvent(fd);
}
void Socket::handleClientFileDescriptorActivity(int index) {
if (events[index].events & EPOLLIN) {
// Invalid filedescriptor
if (events[index].data.fd < 0) {
raiseSocketExceptionEvent("Not a valid filedescriptor.");
return;
}
// Data available for reading
char buffer[512];
ssize_t result = recv(events[index].data.fd, buffer, sizeof(buffer), 0);
if (result < 0) {
if (errno == ECONNRESET) {
close(events[index].data.fd);
raiseSocketExceptionEvent("Failed to receive data");
}
return;
} else if (result == 0) {
close(events[index].data.fd);
raiseClientDisconnectedEvent(events[index].data.fd);
} else {
ev.events = EPOLLOUT | EPOLLIN | EPOLLET;
std::vector<char> data(buffer, buffer + result);
raiseDataReceivedEvent(data , events[index].data.fd);
}
}
if (events[index].events & EPOLLOUT) {
// ToDo Implement
std::cout << "EPOLLOUT" << std::endl;
}
if (events[index].events & EPOLLRDHUP) {
// ToDo Implement
std::cout << "EPOLLRDHUP" << std::endl;
}
if (events[index].events & EPOLLHUP) {
// ToDo Implement
std::cout << "EPOLLHUP" << std::endl;
}
if (events[index].events & EPOLLERR) {
raiseSocketExceptionEvent("Unexpected exception occured EPOLLERR");
close(events[index].data.fd);
}
}
Socket.h
#ifndef TINYSOCKET_SOCKET_H
#define TINYSOCKET_SOCKET_H
#include <sys/socket.h>
#include <netinet/in.h>
#include <sys/epoll.h>
#include <unistd.h>
#include <string>
#include <vector>
#include "ISocketEventListener.h"
#include "SocketExceptionEventArgs.h"
#include "SocketDataEventArgs.h"
#include "SocketConnectEventArgs.h"
class Socket {
public:
/// Initialize new server socket
/// \param port The listening port
Socket(uint16_t port);
/// Initializes the socket, bind the port to the socket etc
/// \return True if initialization was successful, otherewise false,
bool initialize();
/// Start the server socket and listen for events such as incoming and outgoing
/// connections and data as well as exceptions that occurred.
/// \return False if an exception occurred, true if server was stopped on purpose.
bool start();
/// Register a socket event listener
/// \param listener The listener which handles socket events
void addEventListener(ISocketEventListener *listener);
/// Removes a socket event listener
/// \param listener The listener which handles socket events
void removeEventListener(ISocketEventListener *listener);
private:
/// Holds all socket event listeners
std::vector<ISocketEventListener *> eventListeners;
void raiseDataSendEvent(const std::vector<char> &data, int fd) const;
void raiseDataReceivedEvent(const std::vector<char> &data, int fd) const;
void raiseClientConnectedEvent(int socket) const;
void raiseClientDisconnectedEvent(int socket) const;
void raiseSocketExceptionEvent(std::string message) const;
void setNonBlocking(int socket);
void handleListeningFileDescriptorActivity();
void handleClientFileDescriptorActivity(int fd);
const int MAX_EVENTS = 20;
struct epoll_event ev;
struct epoll_event events[20];
const int timeout = -1;
struct sockaddr_in address;
/// The port the server will listen on
uint16_t listeningPort;
/// The socket of the server
int masterSocket;
int socketOptions = 1;
bool running = true;
/// The filedescriptor returned from epoll_create1()
int epollFd;
};
#endif //TINYSOCKET_SOCKET_H
ISocketEventListener.cpp
#include "ISocketEventListener.h"
void ISocketEventListener::onConnect(SocketConnectEventArgs e) {}
void ISocketEventListener::onDisconnect(SocketConnectEventArgs e) {}
void ISocketEventListener::onDataSend(SocketDataEventArgs e) {}
void ISocketEventListener::onDataReceive(SocketDataEventArgs e) {}
void ISocketEventListener::onSocketException(SocketExceptionEventArgs e) {}
ISocketEventListener.h
#ifndef TINYSOCKET_ISOCKETEVENTLISTENER_H
#define TINYSOCKET_ISOCKETEVENTLISTENER_H
#include "SocketConnectEventArgs.h"
#include "SocketDataEventArgs.h"
#include "SocketExceptionEventArgs.h"
class ISocketEventListener {
public:
virtual void onConnect(SocketConnectEventArgs e);
virtual void onDisconnect(SocketConnectEventArgs e);
virtual void onDataSend(SocketDataEventArgs e);
virtual void onDataReceive(SocketDataEventArgs e);
virtual void onSocketException(SocketExceptionEventArgs e);
};
#endif //TINYSOCKET_ISOCKETEVENTLISTENER_H
SocketConnectEventArgs.cpp
#include "SocketConnectEventArgs.h"
SocketConnectEventArgs::SocketConnectEventArgs(int _fd) {
fileDescriptor = _fd;
}
int SocketConnectEventArgs::getFileDescriptor() const {
return fileDescriptor;
}
SocketConnectEventArgs.h
#ifndef TINYSOCKET_SOCKETCONNECTEVENTARGS_H
#define TINYSOCKET_SOCKETCONNECTEVENTARGS_H
class SocketConnectEventArgs {
public:
SocketConnectEventArgs(int _fd);
int getFileDescriptor() const;
private:
int fileDescriptor;
};
#endif //TINYSOCKET_SOCKETCONNECTEVENTARGS_H
SocketDataEventArgs.cpp
#include "SocketDataEventArgs.h"
SocketDataEventArgs::SocketDataEventArgs(std::vector<char> _data, int _fd) {
transfer_data = std::move(_data);
fd = _fd;
}
std::vector<char> SocketDataEventArgs::getData() const {
return transfer_data;
}
int SocketDataEventArgs::getFd() const {
return fd;
}
SocketDataEventArgs.h
#ifndef TINYSOCKET_SOCKETDATAEVENTARGS_H
#define TINYSOCKET_SOCKETDATAEVENTARGS_H
#include <string>
#include <vector>
class SocketDataEventArgs {
public:
SocketDataEventArgs(std::vector<char> _data, int fd);
std::vector<char> getData() const;
int getFd() const;
private:
std::vector<char> transfer_data;
int fd;
};
#endif //TINYSOCKET_SOCKETDATAEVENTARGS_H
SocketExceptionEventArgs.cpp
#include <iostream>
#include "SocketExceptionEventArgs.h"
SocketExceptionEventArgs::SocketExceptionEventArgs(std::string message) {
exceptionMessage = message;
}
const std::string &SocketExceptionEventArgs::getExceptionMessage() const {
return exceptionMessage;
}
SocketExceptionEventArgs.h
#ifndef TINYSOCKET_SOCKETEXCEPTIONEVENTARGS_H
#define TINYSOCKET_SOCKETEXCEPTIONEVENTARGS_H
#include <string>
class SocketExceptionEventArgs {
public:
explicit SocketExceptionEventArgs(std::string message);
const std::string &getExceptionMessage() const;
private:
std::string exceptionMessage;
};
#endif //TINYSOCKET_SOCKETEXCEPTIONEVENTARGS_H
To create a simple server that can handle multiple clients, thee wrapper can be used like this:
main.cpp
#include <iostream>
#include "Socket.h"
class MySocketListener : public ISocketEventListener {
private:
void onConnect(SocketConnectEventArgs e) override {
std::cout << "Client connected | SD: " << e.getFileDescriptor() << std::endl;
}
void onDisconnect(SocketConnectEventArgs e) override {
std::cout << "Client disconnected | SD: " << e.getFileDescriptor() << std::endl;
}
void onDataSend(SocketDataEventArgs e) override {
std::cout << "Client sending data." << std::endl;
}
void onDataReceive(SocketDataEventArgs e) override {
std::vector<char> data = e.getData();
std::cout << "Received from fd " << e.getFd() << ": ";
for (std::vector<char>::const_iterator i = data.begin(); i != data.end(); i++) {
std::cout << *i;
}
std::cout << std::endl;
}
void onSocketException(SocketExceptionEventArgs e) override {
std::cout << "[ERROR] : " << e.getExceptionMessage() << std::endl;
}
};
int main() {
// Declare listener for socket events
auto eventListener = new MySocketListener();
// Create socket object
Socket socket(1337);
// Add event listeners
socket.addEventListener(eventListener);
// Start server
if (socket.initialize()) {
socket.start();
}
return 0;
}
I appreciate all feedback.
-
\$\begingroup\$ Is there a specific reason for you not using c++11 and later versions ? Should an answer stick to pre-c++11 ? \$\endgroup\$Harald Scheirich– Harald Scheirich2018年04月26日 14:14:27 +00:00Commented Apr 26, 2018 at 14:14
-
\$\begingroup\$ @HaraldScheirich I am fine with C++11. However, the post is mainly related to improving the epoll loop/event structure. \$\endgroup\$766F6964– 766F69642018年04月26日 14:22:40 +00:00Commented Apr 26, 2018 at 14:22
-
\$\begingroup\$ In case my post is unclear please let me know. \$\endgroup\$766F6964– 766F69642018年04月27日 09:24:19 +00:00Commented Apr 27, 2018 at 9:24
1 Answer 1
Sorry I can't help much with the socket programming, given the choice i'd probably go for boost::asio
to handle the lower level parts. With regard to your use of epoll
, I looked at the docs and that seems fine.
Single threaded implementation
One of the issues I see with your implementation is that the processing in the listeners is blocking the processing in your poll loop. That means that a if a listener is doing "work" in it's event notification callback, the event polling work stops. You might not need this in your specific application but I'd probably would like to see the polling and reception of data, and the event processing done in two separate threads.
This could be accomplished by the listener thread pushing the events onto an event queue, and a separate thread dealing with the listeners in it's own loop whenever there is work left. C++11 with std::thread
and std::mutex
would be helpful here.
Listener Interfaces
Your listener interface has different call signatures for each event that is fine, but wrapping the event data in separate structures seems over the top when you already have separate interfaces. E.g. the SocketConnectEventArgs
has one member, the file descriptor. There is no reason for having that wrapped in a class. That lister interface might as well look like this
class ISocketEventListener {
public:
virtual void onConnect(int fd);
virtual void onDisconnect(int fd);
virtual void onDataSend(int fd, const std::vector<char>& data);
virtual void onDataReceive(int fd, const std::vector<char>& data);
virtual void onSocketException(const std::string& exception);
};
Wrapping the arguments in a class would make more sense if you used one listener call signature. This might become necessary if you implement something like the queuing construct mentioned above. But then the class would probably be structured so you could push all events onto one queue. (Mainly so wouldn't have to implement 5 different queues).
If nothing else the event objects should be passed via const &
e.g.
virtual void onConnect(const SocketConnectEventArgs& event);
especially for the data events this will prevent copying the data that is being passed.
Sharing raw pointers
Your listeners get registered via raw pointers, this can lead to problems when you don't know who is holding on to these pointers and who is responsible for cleaning them up. This is a case where std::shared_ptr
and std::weak_ptr
are really helpful, depending on how you envision the ownership of the listeners. If your Socket
class holds the listener via std::weak_ptr
the could be deleted from the outside and the socket class could check if they are still alive. If you want the Socket class to co-own the listeners use shared:ptr
and they would stay alive as long as the socket is up.
Range based for
loops
In C++11 and above its much easier to write for loops, and more idiomatic to use range based for loops, your event broadcast loops could look like this
for(auto& listener : eventListeners) {
listener.onConnect(fd);
}
Destructor
You didn't write what you want to do with this, Socket
is a class that doesn't seemed to be designed for ever being shut down. Which might be fine for your use. I'd probably still like to see an orderly shutdown for the connection, if nothing else so you don't have to research socket programming again when you actually do need it.
-
\$\begingroup\$ Thanks for the review. It certainly does include some valid points. I wonder tho if the epoll loop structure can be improved somehow. It looks really messy on my code. \$\endgroup\$766F6964– 766F69642018年04月30日 23:01:15 +00:00Commented Apr 30, 2018 at 23:01
-
\$\begingroup\$ Which parts do you perceive as messy ? You are doing fairly low level programming here, a lot of times that doesn't necessarily lend it self to looking "pretty". Overall the loop looks fairly reasonable, you seem to handle a good bunch of error cases. The only other prettyfication i saw is using the event rather than an index as input to
handleClientFileDescriptorActivity()
\$\endgroup\$Harald Scheirich– Harald Scheirich2018年05月01日 14:27:06 +00:00Commented May 1, 2018 at 14:27