mirror of
https://codeberg.org/dnkl/foot.git
synced 2026-02-05 04:06:08 -05:00
fdm: don't free FD data that may be referenced by epoll return data
It is perfectly possible, and legal, for a FDM handler to delete another handler. The problem however is when the epoll returned array of FD events contain the removed FD handler *after* the handler that removed it. That is, if the epoll returned array is: [FD=13, FD=37] and the handler for FD=13 removes FD=37, then given the current implementation, the FD user data (our handler callback etc) will point to a free:d address. Add support for this situation by deferring FD handler removal *when called from another handler*. This is done by "locking" the FDM struct before looping the handlers for FDs with events (and unlocking afterwards). In fdm_del(), we check if the FDM has been locked, in which case the FD is marked as deleted, and put on a deferred list. But not *actually* deleted. Meanwhile, in the FDM poll function, we skip calling handlers for marked-for-deletion FDs. Then, when all FDs have been processed, we loop the deferred list and finally deletes the FDs for real.
This commit is contained in:
parent
4ae22b72a1
commit
80c4721e57
2 changed files with 60 additions and 9 deletions
68
fdm.c
68
fdm.c
|
|
@ -1,6 +1,7 @@
|
|||
#include "fdm.h"
|
||||
|
||||
#include <stdlib.h>
|
||||
#include <stdbool.h>
|
||||
#include <unistd.h>
|
||||
#include <errno.h>
|
||||
#include <assert.h>
|
||||
|
|
@ -15,11 +16,14 @@ struct fd {
|
|||
int fd;
|
||||
void *data;
|
||||
fdm_handler_t handler;
|
||||
bool deleted;
|
||||
};
|
||||
|
||||
struct fdm {
|
||||
int epoll_fd;
|
||||
tll(struct fd) fds;
|
||||
bool is_polling;
|
||||
tll(struct fd *) fds;
|
||||
tll(struct fd *) deferred_delete;
|
||||
};
|
||||
|
||||
struct fdm *
|
||||
|
|
@ -34,7 +38,9 @@ fdm_init(void)
|
|||
struct fdm *fdm = malloc(sizeof(*fdm));
|
||||
*fdm = (struct fdm){
|
||||
.epoll_fd = epoll_fd,
|
||||
.is_polling = false,
|
||||
.fds = tll_init(),
|
||||
.deferred_delete = tll_init(),
|
||||
};
|
||||
return fdm;
|
||||
}
|
||||
|
|
@ -60,23 +66,31 @@ fdm_add(struct fdm *fdm, int fd, int events, fdm_handler_t handler, void *data)
|
|||
{
|
||||
#if defined(_DEBUG)
|
||||
tll_foreach(fdm->fds, it) {
|
||||
if (it->item.fd == fd) {
|
||||
if (it->item->fd == fd) {
|
||||
LOG_ERR("FD=%d already registered", fd);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
#endif
|
||||
|
||||
tll_push_back(
|
||||
fdm->fds, ((struct fd){.fd = fd, .data = data, .handler = handler}));
|
||||
struct fd *fd_data = malloc(sizeof(*fd_data));
|
||||
*fd_data = (struct fd) {
|
||||
.fd = fd,
|
||||
.data = data,
|
||||
.handler = handler,
|
||||
.deleted = false,
|
||||
};
|
||||
|
||||
tll_push_back(fdm->fds, fd_data);
|
||||
|
||||
struct epoll_event ev = {
|
||||
.events = events,
|
||||
.data = {.ptr = &tll_back(fdm->fds)},
|
||||
.data = {.ptr = fd_data},
|
||||
};
|
||||
|
||||
if (epoll_ctl(fdm->epoll_fd, EPOLL_CTL_ADD, fd, &ev) < 0) {
|
||||
LOG_ERRNO("failed to register FD with epoll");
|
||||
free(fd_data);
|
||||
tll_pop_back(fdm->fds);
|
||||
return false;
|
||||
}
|
||||
|
|
@ -84,14 +98,26 @@ fdm_add(struct fdm *fdm, int fd, int events, fdm_handler_t handler, void *data)
|
|||
return true;
|
||||
}
|
||||
|
||||
bool
|
||||
fdm_del(struct fdm *fdm, int fd)
|
||||
static bool
|
||||
fdm_del_internal(struct fdm *fdm, int fd, bool close_fd)
|
||||
{
|
||||
if (fd == -1)
|
||||
return true;
|
||||
|
||||
tll_foreach(fdm->fds, it) {
|
||||
if (it->item.fd == fd) {
|
||||
if (it->item->fd == fd) {
|
||||
if (epoll_ctl(fdm->epoll_fd, EPOLL_CTL_DEL, fd, NULL) < 0)
|
||||
LOG_ERRNO("failed to unregister FD=%d from epoll", fd);
|
||||
|
||||
if (close_fd)
|
||||
close(it->item->fd);
|
||||
|
||||
it->item->deleted = true;
|
||||
if (fdm->is_polling)
|
||||
tll_push_back(fdm->deferred_delete, it->item);
|
||||
else
|
||||
free(it->item);
|
||||
|
||||
tll_remove(fdm->fds, it);
|
||||
return true;
|
||||
}
|
||||
|
|
@ -101,6 +127,18 @@ fdm_del(struct fdm *fdm, int fd)
|
|||
return false;
|
||||
}
|
||||
|
||||
bool
|
||||
fdm_del(struct fdm *fdm, int fd)
|
||||
{
|
||||
return fdm_del_internal(fdm, fd, true);
|
||||
}
|
||||
|
||||
bool
|
||||
fdm_del_no_close(struct fdm *fdm, int fd)
|
||||
{
|
||||
return fdm_del_internal(fdm, fd, false);
|
||||
}
|
||||
|
||||
bool
|
||||
fdm_poll(struct fdm *fdm)
|
||||
{
|
||||
|
|
@ -114,10 +152,22 @@ fdm_poll(struct fdm *fdm)
|
|||
return false;
|
||||
}
|
||||
|
||||
fdm->is_polling = true;
|
||||
for (int i = 0; i < ret; i++) {
|
||||
struct fd *fd = events[i].data.ptr;
|
||||
if (!fd->handler(fdm, fd->fd, events[i].events, fd->data))
|
||||
if (fd->deleted)
|
||||
continue;
|
||||
|
||||
if (!fd->handler(fdm, fd->fd, events[i].events, fd->data)) {
|
||||
fdm->is_polling = false;
|
||||
return false;
|
||||
}
|
||||
}
|
||||
fdm->is_polling = false;
|
||||
|
||||
tll_foreach(fdm->deferred_delete, it) {
|
||||
free(it->item);
|
||||
tll_remove(fdm->deferred_delete, it);
|
||||
}
|
||||
|
||||
return true;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue