From 4d9db4c30f339689df7581087adf414b58b29d2b Mon Sep 17 00:00:00 2001 From: Arne Goedeke <el@laramies.com> Date: Sun, 24 Feb 2013 00:46:35 +0100 Subject: [PATCH] System.Inotify: use files.Fd() objects instead of file descriptors --- .../Filesystem.pmod/Monitor.pmod/basic.pike | 3 +- lib/modules/System.pmod/Inotify.pmod | 12 +++-- src/modules/Inotify/inotify.cmod | 49 ++++++++++++++++--- src/modules/Inotify/testsuite.in | 3 +- 4 files changed, 56 insertions(+), 11 deletions(-) diff --git a/lib/modules/Filesystem.pmod/Monitor.pmod/basic.pike b/lib/modules/Filesystem.pmod/Monitor.pmod/basic.pike index 87ae1d4e7c..689dfcee1a 100644 --- a/lib/modules/Filesystem.pmod/Monitor.pmod/basic.pike +++ b/lib/modules/Filesystem.pmod/Monitor.pmod/basic.pike @@ -780,7 +780,8 @@ protected void create(int|void max_dir_check_interval, eventstream->callback_func = eventstream_callback; #elseif HAVE_INOTIFY instance = Inotify._Instance(); - file = Stdio.File(instance->get_fd(), "r"); + file = Stdio.File(); + file->assign(instance->fd()); file->set_nonblocking(); file->set_read_callback(inotify_parse); #endif diff --git a/lib/modules/System.pmod/Inotify.pmod b/lib/modules/System.pmod/Inotify.pmod index 4eef892a98..98ee2acc79 100644 --- a/lib/modules/System.pmod/Inotify.pmod +++ b/lib/modules/System.pmod/Inotify.pmod @@ -107,7 +107,8 @@ class Instance { void create() { instance = _Instance(); - file = Stdio.File(instance->get_fd(), "r"); + file = Stdio.File(); + file->assign(instance->fd()); file->set_nonblocking(); file->set_read_callback(parse_fun(watches)); } @@ -168,8 +169,13 @@ class Instance { } void destroy() { - destruct(file); - destruct(instance); + file->set_read_callback(0); + /* + * closing the last copy of an inotify fd currently takes a long time (~100ms). + * make sure to close the file first, to allow releasing the interpreter lock + * during the last close call in the instance EXIT handler. + */ + file->close(); } } #endif diff --git a/src/modules/Inotify/inotify.cmod b/src/modules/Inotify/inotify.cmod index 934caee797..864635e3c9 100644 --- a/src/modules/Inotify/inotify.cmod +++ b/src/modules/Inotify/inotify.cmod @@ -25,11 +25,16 @@ #include "object.h" #include "pike_types.h" #include "builtin_functions.h" +#include "fdlib.h" +#include "pike_threadlib.h" + +#include "modules/files/file.h" #ifdef HAVE_SYS_INOTIFY_H #include <sys/inotify.h> #include <errno.h> +#include <unistd.h> DECLARATIONS @@ -147,6 +152,7 @@ PIKEFUN array(string|int) parse_event(string data) { */ PIKECLASS _Instance { CVAR int fd; + CVAR struct object * fd_object; /*! @decl int add_watch(string file, int mask) *! Add a watch for a certain file or directory and specific events. @@ -187,13 +193,24 @@ PIKECLASS _Instance { RETURN err; } - /*! @decl int get_fd() + + /*! @decl object get_fd() *! @returns - *! Returns the file descriptor that is associated to the inotify - *! instance. + *! Returns the file descriptor associated with this inotify instance. + *! @note + *! Use @[fd()] instead. */ PIKEFUN int get_fd() { - RETURN THIS->fd; + push_int(THIS->fd); + } + + /*! @decl object fd() + *! @returns + *! Returns a instance of @[Stdio.Fd] corresponding to the inotify instance. This can passed to + *! @[Stdio.File()->assign()]. + */ + PIKEFUN object fd() { + ref_push_object(THIS->fd_object); } /*! @decl int rm_watch(int wd) @@ -218,19 +235,39 @@ PIKECLASS _Instance { } INIT { + struct object * o; THIS->fd = inotify_init(); + THIS->fd_object = NULL; if (THIS->fd == -1) switch (errno) { case EMFILE: Pike_error("User limit on inotify instances reached.\n"); case ENFILE: Pike_error("User limit on file descriptors reached.\n"); - case ENOMEM: Pike_error("No free kernel memory available.\n"); + case ENOMEM: + Pike_error("No free kernel memory available.\n"); } + + o = file_make_object_from_fd(THIS->fd, FILE_READ, fd_CAN_NONBLOCK); + /* We will close the inotify fd on EXIT */ + ((struct my_file *)(o->storage + o->prog->inherits->storage_offset))->flags |= FILE_NO_CLOSE_ON_DESTRUCT; + THIS->fd_object = o; } EXIT { - close(THIS->fd); + if (THIS->fd_object) { + free_object(THIS->fd_object); + THIS->fd_object = NULL; + } + if (THIS->fd != -1) { + int fd = THIS->fd; + /* + * currently (linux 3.4.9) closing an inotify fd takes in the order of 100 ms + */ + THREADS_ALLOW(); + close(fd); + THREADS_DISALLOW(); + } } } diff --git a/src/modules/Inotify/testsuite.in b/src/modules/Inotify/testsuite.in index 2d10b02dd6..b461ef164f 100644 --- a/src/modules/Inotify/testsuite.in +++ b/src/modules/Inotify/testsuite.in @@ -43,7 +43,8 @@ test_any([[ // stole this from pikes Stdio.File testsuite. ]], 1) test_any([[ object i = System.Inotify._Instance(); - object file = Stdio.File(i->get_fd(), "r"); + object file = Stdio.File(); + file->assign(i->fd()); i->add_watch(testdir, System.Inotify.IN_CREATE); Stdio.write_file(testfile, "test"); array a = System.Inotify.parse_event(file->read(4096, 1)); -- GitLab