I am writing an epoll based TCP server in c++, and I am currently representing new connections as an object:
Connection::Connection(const int epoll, const int socket): m_socket{socket}, m_epoll{epoll} {
int flags = fcntl(m_socket, F_GETFL, 0);
if (flags == -1) {
throw std::system_error(errno, std::system_category(), "Connection::Connection fcntl");
}
flags |= O_NONBLOCK;
if (fcntl(m_socket, F_SETFL, flags) == -1) {
throw std::system_error(errno, std::system_category(), "Connection::Connection fcntl");
}
epoll_event event{};
event.events = EPOLLIN;
event.data.ptr = this;
if (epoll_ctl(m_epoll, EPOLL_CTL_ADD, m_socket, &event) == -1) {
throw std::system_error(errno, std::system_category(), "Connection::Connection epoll_ctl");
}
}
Connection::~Connection() {
if (epoll_ctl(m_epoll, EPOLL_CTL_DEL, m_socket, nullptr) == -1) {
throw std::system_error(errno, std::system_category(), "Connection::~Connection epoll_ctl");
}
if (close(m_socket) == -1) {
throw std::system_error(errno, std::system_category(), "Connection::~Connection close");
}
}
void Connection::handle_receive() {
const ssize_t bytes_received = recv(m_socket, m_read_buffer.data(), buffer_size, 0);
if (bytes_received == -1) {
// if recv fails for normal reason (read not actually available)
if (errno == EAGAIN || errno == EWOULDBLOCK) {
return;
}
}
// either the connection was closed gracefully by the client, or the connection is broken in some way
// either way we want to delete the connection
if (bytes_received <= 0) {
delete this;
}
}
The main thing to look out for here is the use of delete this within handle_read, which is called whenever a read is available on the socket. While delete this can be dangerous in itself, I think it is fine in this scenario as I am certain that in my use case Connection is a heap allocated object which (hopefully) shouldn't be referenced again. The thing that I am more concerned about is that, after the memory for the connection is deleted, there could exist other events further back in the queue which reference the same Connection, and thus will result in a use after free when they are processed. If this is the case, I am fairly certain that even calling delete from outside of Connection is unsafe. That being said, my question is whether this is a scenario that could occur.
I assume you have something like EpollDispatcher object where you call epoll_wait and handle_receive for your Connection. This means what you should have a reference to Connection there and so delete this is forbidden for you.
The main rule is to call new and delete only in object owns the resource. You should define an owner of Connection and the owner should manage it's lifetime, probably with help of std::unique_ptr.
Let's think of EpollDispatcher being the owner.
class EpollDispatcher {
std::map<int, std::unique_ptr<Connection>> connections;
int epoll_fd;
public:
EpollDispatcher(){
... // initialize epoll_fd
}
void AddConnection(int socket){
connections.emplace(socket, std::make_unique<Connection>(epoll_fd, socket));
}
void Wait(){
std::array<struct epoll_event, kMaxEvents> events;
int res = epoll_wait(epoll_fd_, events.data(), events.size(), -1);
if (res < 0){
... // error handling
}
for (std::size_t i = 0; i < static_cast<std::size_t>(res); ++i)
{
auto& event = events[i];
auto conn_it = connections.find(event.data.fd);
if (conn_it == std::end(connections)){
continue;
}
if ((event.events & EPOLLIN) != 0) {
if (!conn_it->second->handle_receive()){
connections.erase(conn_it);
continue;
}
}
if ((event.events & EPOLLOUT) != 0) {
if (!conn_it->second->handle_send()){
connections.erase(conn_it);
continue;
}
}
... // handle other events & FLAG
}
}
};
And handle receive should be
bool Connection::handle_receive() {
const ssize_t bytes_received = recv(m_socket, m_read_buffer.data(), buffer_size, 0);
if (bytes_received == -1) {
// if recv fails for normal reason (read not actually available)
if (errno == EAGAIN || errno == EWOULDBLOCK) {
return true;
}
}
// either the connection was closed gracefully by the client, or the connection is broken in some way
// either way we want to delete the connection
if (bytes_received <= 0) {
return false;
}
}
... handle_send with same logic.
You can also replace return bool to through the exception and catching it in EpollDispatcher::Wait.
This code snippets doesn't cover sending and handling received data. I have used event system in my project for handling such events. https://github.com/aethernetio/aether-client-cpp/blob/main/aether/poller/epoll_poller.cpp#L192
Connection (In my case ITransport) object subscribed to poller events and handle it inside itself, all the data and errors emitted by ITransport' events. And the owner of std::unique_ptr<ITransport> decides when to delete it.