Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement POSIX polling dirmonitor #1926

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion meson_options.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ option('bundle', type : 'boolean', value : false, description: 'Build a macOS bu
option('source-only', type : 'boolean', value : false, description: 'Configure source files only, doesn\'t checks for dependencies')
option('portable', type : 'boolean', value : false, description: 'Portable install')
option('renderer', type : 'boolean', value : false, description: 'Use SDL renderer')
option('dirmonitor_backend', type : 'combo', value : '', choices : ['', 'inotify', 'fsevents', 'kqueue', 'win32', 'dummy'], description: 'define what dirmonitor backend to use')
option('dirmonitor_backend', type : 'combo', value : '', choices : ['', 'inotify', 'fsevents', 'kqueue', 'win32', 'posix_polling', 'dummy'], description: 'define what dirmonitor backend to use')
option('arch_tuple', type : 'string', value : '', description: 'Specify a custom architecture tuple')
option('use_system_lua', type : 'boolean', value : false, description: 'Prefer System Lua over a the meson wrap')
142 changes: 142 additions & 0 deletions src/api/dirmonitor/posix_polling.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
#include <stdlib.h>
#include <string.h>
#include <fcntl.h>
#include <unistd.h>
#include <sys/stat.h>
#include <time.h>
#include <SDL.h>

#define EVENT_MODIFIED (1 << 0)

#if _BSD_SOURCE || _SVID_SOURCE || _XOPEN_SOURCE > 700 || _POSIX_C_SOURCE >= 200809L
#define STAT_HAS_TIM
#endif

struct file_info {
int fd;
dev_t dev;
ino_t ino;
struct timespec mtim;
int events;
};

struct dirmonitor_internal {
struct file_info* fds;
size_t fd_count;
};


struct dirmonitor_internal* init_dirmonitor() {
struct dirmonitor_internal* monitor = calloc(1, sizeof(struct dirmonitor_internal));

monitor->fd_count = 0;
monitor->fds = NULL;
return monitor;
}

void deinit_dirmonitor(struct dirmonitor_internal* monitor) {
for (size_t i = 0; i < monitor->fd_count; ++i)
close(monitor->fds[i].fd);
free(monitor->fds);
}

int get_changes_dirmonitor(struct dirmonitor_internal* monitor, char* buffer, int len)
{
struct stat new_stat;
int c = 0;

for (size_t i = 0; i < monitor->fd_count; ++i) {
if (monitor->fds[i].fd < 0)
continue;
Comment on lines +48 to +50
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could fd_count be raced over?
Same with the fds structs.

For example:

  1. We enter get_changes_dirmonitor
  2. Main thread enters add_dirmonitor
  3. add_dirmonitor needs to allocate new space, so it does, but still doesn't switch it out
  4. get_changes_dirmonitor saves fds in a register
  5. add_dirmonitor switches out the spaces, frees the old fds
  6. get_changes_dirmonitor uses the fds from the register, SIGSEGVs.


fstat(monitor->fds[i].fd, &new_stat);

if (
new_stat.st_mtime > monitor->fds[i].mtim.tv_sec ||
#ifdef STAT_HAS_TIM
(new_stat.st_mtim.tv_sec == monitor->fds[i].mtim.tv_sec && new_stat.st_mtim.tv_nsec > monitor->fds[i].mtim.tv_nsec) ||
#endif
new_stat.st_dev != monitor->fds[i].dev || // device changed
new_stat.st_ino != monitor->fds[i].ino || // inode changed
0
) {
monitor->fds[i].mtim.tv_sec = new_stat.st_mtime;
#ifdef STAT_HAS_TIM
monitor->fds[i].mtim.tv_nsec = new_stat.st_mtim.tv_nsec;
#endif
monitor->fds[i].dev = new_stat.st_dev;
monitor->fds[i].ino = new_stat.st_ino;
monitor->fds[i].events |= EVENT_MODIFIED;
c += 1;
}
}

if (!c)
SDL_Delay(100);

return c;
}

int translate_changes_dirmonitor(struct dirmonitor_internal* monitor, char* buffer, int count, int (*change_callback)(int, const char*, void*), void* data) {
for (size_t i = 0; i < monitor->fd_count; ++i) {
if (monitor->fds[i].events & EVENT_MODIFIED) {
--count;
monitor->fds[i].events = monitor->fds[i].events & ~ EVENT_MODIFIED;
change_callback(monitor->fds[i].fd, NULL, data);
}
}

return 0;
}

int add_dirmonitor(struct dirmonitor_internal* monitor, const char* path)
{
int fd = open(path, O_RDONLY | O_NOFOLLOW);
if (fd == -1)
return -1;

// Check if there is any space we can reuse
for (size_t i = 0; i < monitor->fd_count; ++i) {
if (monitor->fds[i].fd == -1) {
monitor->fds[i].fd = fd;
monitor->fds[i].mtim.tv_sec = 0;
return fd;
}
}

// There is no empty space, add a new entry;
// Prepare a new list before switching it to prevent race conditions
struct file_info* old_fds = monitor->fds;
struct file_info* new_fds;
new_fds = malloc(sizeof(*new_fds) * (monitor->fd_count + 1));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not too sure about just allocating one more space.

if (old_fds)
memcpy(new_fds, old_fds, sizeof(*new_fds) * (monitor->fd_count));

new_fds[monitor->fd_count].fd = fd;
new_fds[monitor->fd_count].mtim.tv_sec = 0;

monitor->fds = new_fds;
free(old_fds);

monitor->fd_count++;

return fd;
}

void remove_dirmonitor(struct dirmonitor_internal* monitor, int fd)
{
for (size_t i = 0; i < monitor->fd_count; ++i) {
if (monitor->fds[i].fd == fd) {
int fd = monitor->fds[i].fd;

monitor->fds[i].fd = -1;
monitor->fds[i].events = 0;
close(fd);
return;
}
}
}

int get_mode_dirmonitor() {
return 2;
}
4 changes: 4 additions & 0 deletions src/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ if get_option('dirmonitor_backend') == ''
dirmonitor_backend = 'kqueue'
elif host_machine.system() == 'windows'
dirmonitor_backend = 'win32'
elif cc.has_header('sys/stat.h')
dirmonitor_backend = 'posix_polling'
else
dirmonitor_backend = 'dummy'
warning('no suitable backend found, defaulting to dummy backend')
Expand All @@ -51,6 +53,8 @@ elif dirmonitor_backend == 'inodewatcher'
lite_sources += 'api/dirmonitor/inodewatcher.cpp'
elif dirmonitor_backend == 'win32'
lite_sources += 'api/dirmonitor/win32.c'
elif dirmonitor_backend == 'posix_polling'
lite_sources += 'api/dirmonitor/posix_polling.c'
else
lite_sources += 'api/dirmonitor/dummy.c'
endif
Expand Down