Mirrors/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell synced 2024-12-26 12:53:13 +00:00
Code Issues Projects Releases Packages Wiki Activity

Remove inotify-based universal notifier

Browse source 
The inotify notifier is fragile, fails on travis, and fails to compile
on certain Linux kernels. It doesn't appear to work as well as the named
pipe mechanism. Best to just get rid of it.
This commit is contained in:
ridiculousfish 2014-05-29 09:57:56 -07:00
parent 655293ece9
commit 9ae06c541f
5 changed files with 1 additions and 223 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
configure.ac
View file

@ -536,7 +536,6 @@ AC_CHECK_FUNCS( wcsdup wcsndup wcslen wcscasecmp wcsncasecmp fwprintf )
AC_CHECK_FUNCS( futimes wcwidth wcswidth wcstok fputwc fgetwc ) AC_CHECK_FUNCS( futimes wcwidth wcswidth wcstok fputwc fgetwc )
AC_CHECK_FUNCS( wcstol wcslcat wcslcpy lrand48_r killpg mkostemp ) AC_CHECK_FUNCS( wcstol wcslcat wcslcpy lrand48_r killpg mkostemp )
AC_CHECK_FUNCS( backtrace backtrace_symbols sysconf getifaddrs getpeerucred getpeereid ) AC_CHECK_FUNCS( backtrace backtrace_symbols sysconf getifaddrs getpeerucred getpeereid )
AC_CHECK_FUNCS( inotify_init inotify_init1 )
if test x$local_gettext != xno; then if test x$local_gettext != xno; then
AC_CHECK_FUNCS( gettext dcgettext ) AC_CHECK_FUNCS( gettext dcgettext )

6
env_universal.cpp
View file

@ -285,11 +285,6 @@ static void reconnect()
} }
} }
void env_universal_read_from_file()
{
}
void env_universal_init(wchar_t * p, void env_universal_init(wchar_t * p,
wchar_t *u, wchar_t *u,
void (*sf)(), void (*sf)(),
@ -299,7 +294,6 @@ void env_universal_init(wchar_t * p,
{ {
external_callback = cb; external_callback = cb;
env_universal_common_init(&callback); env_universal_common_init(&callback);
env_universal_read_from_file();
s_env_univeral_inited = true; s_env_univeral_inited = true;
} }
else else

112
env_universal_common.cpp
View file

@ -53,11 +53,6 @@
#include <notify.h> #include <notify.h>
#endif #endif
#if HAVE_INOTIFY_INIT || HAVE_INOTIFY_INIT1
#define FISH_INOTIFY_AVAILABLE 1
#include <sys/inotify.h>
#endif
/** /**
Non-wide version of the set command Non-wide version of the set command
*/ */
@ -1622,109 +1617,6 @@ public:
} }
}; };
/* inotify-based notifier. This attempts to watch the uvars file directly. Since the file is never modified (only replaced), it has to watch for deletions. If the file is not present, this will probably break horribly. Needs love before it can be competitive. */
class universal_notifier_inotify_t : public universal_notifier_t
{
int watch_fd;
int watch_descriptor;
const std::string narrow_path;
void reestablish_watch()
{
#if FISH_INOTIFY_AVAILABLE
if (this->watch_fd >= 0)
{
if (this->watch_descriptor >= 0)
{
inotify_rm_watch(this->watch_fd, this->watch_descriptor);
}
this->watch_descriptor = inotify_add_watch(this->watch_fd, narrow_path.c_str(), IN_MODIFY | IN_MOVE_SELF | IN_DELETE_SELF | IN_EXCL_UNLINK);
if (this->watch_descriptor < 0)
{
wperror(L"inotify_add_watch");
}
}
#endif
}
void setup_inotify(const wchar_t *test_path)
{
#if FISH_INOTIFY_AVAILABLE
// Construct the watchfd
#if HAVE_INOTIFY_INIT1
this->watch_fd = inotify_init1(IN_NONBLOCK | IN_CLOEXEC);
#else
this->watch_fd = inotify_init();
#endif
if (this->watch_fd < 0)
{
wperror(L"inotify_init");
}
else
{
int flags = fcntl(this->watch_fd, F_GETFL, 0);
fcntl(this->watch_fd, F_SETFL, flags | O_NONBLOCK | FD_CLOEXEC);
}
reestablish_watch();
#endif
}
public:
universal_notifier_inotify_t(const wchar_t *test_path) : watch_fd(-1), watch_descriptor(-1), narrow_path(wcs2string(test_path ? test_path : default_vars_path()))
{
setup_inotify(test_path);
}
~universal_notifier_inotify_t()
{
if (watch_fd >= 0)
{
close(watch_fd);
}
USE(watch_descriptor);
}
int notification_fd()
{
return watch_fd;
}
bool notification_fd_became_readable(int fd)
{
assert(fd == watch_fd);
bool result = false;
#if FISH_INOTIFY_AVAILABLE
for (;;)
{
struct inotify_event evt = {};
ssize_t read_amt = read(watch_fd, &evt, sizeof evt);
if (read_amt >= (ssize_t)sizeof evt)
{
if (evt.mask & (IN_DELETE_SELF | IN_MOVE_SELF))
{
// When a file is deleted, the watch is lost. Recreate it.
reestablish_watch();
result = true;
}
if (evt.mask & IN_MODIFY)
{
result = true;
}
}
else
{
break;
}
}
#endif
return result;
}
};
#define NAMED_PIPE_FLASH_DURATION_USEC (1000000 / 10) #define NAMED_PIPE_FLASH_DURATION_USEC (1000000 / 10)
#define SUSTAINED_READABILITY_CLEANUP_DURATION_USEC (1000000 * 5) #define SUSTAINED_READABILITY_CLEANUP_DURATION_USEC (1000000 * 5)
@ -1960,7 +1852,6 @@ static universal_notifier_t::notifier_strategy_t fetch_default_strategy_from_env
{"default", universal_notifier_t::strategy_default}, {"default", universal_notifier_t::strategy_default},
{"shmem", universal_notifier_t::strategy_shmem_polling}, {"shmem", universal_notifier_t::strategy_shmem_polling},
{"pipe", universal_notifier_t::strategy_named_pipe}, {"pipe", universal_notifier_t::strategy_named_pipe},
{"inotify", universal_notifier_t::strategy_inotify},
{"notifyd", universal_notifier_t::strategy_notifyd} {"notifyd", universal_notifier_t::strategy_notifyd}
}; };
const size_t opt_count = sizeof options / sizeof *options; const size_t opt_count = sizeof options / sizeof *options;
@ -2025,9 +1916,6 @@ universal_notifier_t *universal_notifier_t::new_notifier_for_strategy(universal_
case strategy_notifyd: case strategy_notifyd:
return new universal_notifier_notifyd_t(); return new universal_notifier_notifyd_t();
case strategy_inotify:
return new universal_notifier_inotify_t(test_path);
case strategy_named_pipe: case strategy_named_pipe:
return new universal_notifier_named_pipe_t(test_path); return new universal_notifier_named_pipe_t(test_path);

5
env_universal_common.h
View file

@ -297,10 +297,7 @@ public:
// Strategy that uses a named pipe. Somewhat complex, but portable and doesn't require polling most of the time. // Strategy that uses a named pipe. Somewhat complex, but portable and doesn't require polling most of the time.
strategy_named_pipe, strategy_named_pipe,
// Strategy that attempts to detect file changes directly via inotify. Doesn't handle certain edge cases (like the file missing); not yet recommended. Linux only.
strategy_inotify,
// Strategy that uses notify(3). Simple and efficient, but OS X only. // Strategy that uses notify(3). Simple and efficient, but OS X only.
strategy_notifyd, strategy_notifyd,

100
fish_tests.cpp
View file

@ -65,12 +65,6 @@
#include "utf8.h" #include "utf8.h"
#include "env_universal_common.h" #include "env_universal_common.h"
#if HAVE_INOTIFY_INIT || HAVE_INOTIFY_INIT1
#include <sys/inotify.h>
#include <sys/utsname.h>
#endif
static const char * const * s_arguments; static const char * const * s_arguments;
static int s_test_run_count = 0; static int s_test_run_count = 0;
@ -2309,16 +2303,6 @@ static void trigger_or_wait_for_notification(universal_notifier_t *notifier, uni
case universal_notifier_t::strategy_named_pipe: case universal_notifier_t::strategy_named_pipe:
case universal_notifier_t::strategy_null: case universal_notifier_t::strategy_null:
break; break;
case universal_notifier_t::strategy_inotify:
{
// Hacktastic. Replace the file, then wait
char cmd[512];
sprintf(cmd, "touch %ls ; mv %ls %ls", UVARS_TEST_PATH L".tmp", UVARS_TEST_PATH ".tmp", UVARS_TEST_PATH);
if (system(cmd)) err(L"Command failed: %s", cmd);
usleep(1000000 / 25);
break;
}
} }
} }
@ -2397,82 +2381,6 @@ static void test_notifiers_with_strategy(universal_notifier_t::notifier_strategy
} }
} }
#if HAVE_INOTIFY_INIT
#define INOTIFY_TEST_PATH "/tmp/inotify_test.tmp"
__attribute__((unused))
static bool test_basic_inotify_support()
{
bool inotify_works = true;
int fd = inotify_init();
if (fd < 0)
{
err(L"inotify_init failed");
}
/* Mark fd as nonblocking */
int flags = fcntl(fd, F_GETFL, 0);
if (flags == -1)
{
err(L"fcntl GETFL failed");
}
if (fcntl(fd, F_SETFL, flags | O_NONBLOCK) == -1)
{
err(L"fcntl SETFL failed");
}
if (system("touch " INOTIFY_TEST_PATH))
{
err(L"touch failed");
}
/* Add file to watch list */
int wd = inotify_add_watch(fd, INOTIFY_TEST_PATH, IN_DELETE | IN_DELETE_SELF);
if (wd < 0)
{
err(L"inotify_add_watch failed: %s", strerror(errno));
}
/* Delete file */
if (system("rm " INOTIFY_TEST_PATH))
{
err(L"rm failed");
}
/* Verify that file is deleted */
struct stat statbuf;
if (stat(INOTIFY_TEST_PATH, &statbuf) != -1 || errno != ENOENT)
{
err(L"File at path " INOTIFY_TEST_PATH " still exists after deleting it");
}
/* The fd should be readable now or very shortly */
struct timeval tv = {1, 0};
fd_set fds;
FD_ZERO(&fds);
FD_SET(fd, &fds);
int count = select(fd + 1, &fds, NULL, NULL, &tv);
if (count == 0 || ! FD_ISSET(fd, &fds))
{
inotify_works = false;
err(L"inotify file descriptor not readable. Is inotify busted?");
struct utsname version = {};
uname(&version);
fprintf(stderr, "kernel version %s - %s\n", version.release, version.version);
}
else
{
fprintf(stderr, "inotify seems OK\n");
}
if (fd >= 0)
{
close(fd);
}
return inotify_works;
}
#endif
static void test_universal_notifiers() static void test_universal_notifiers()
{ {
if (system("mkdir -p /tmp/fish_uvars_test/ && touch /tmp/fish_uvars_test/varsfile.txt")) err(L"mkdir failed"); if (system("mkdir -p /tmp/fish_uvars_test/ && touch /tmp/fish_uvars_test/varsfile.txt")) err(L"mkdir failed");
@ -2482,14 +2390,6 @@ static void test_universal_notifiers()
test_notifiers_with_strategy(universal_notifier_t::strategy_notifyd); test_notifiers_with_strategy(universal_notifier_t::strategy_notifyd);
#endif #endif
// inotify test disabled pending investigation into why this fails on travis-ci
// https://github.com/travis-ci/travis-ci/issues/2342
#if 0
#if HAVE_INOTIFY_INIT
test_basic_inotify_support();
test_notifiers_with_strategy(universal_notifier_t::strategy_inotify);
#endif
#endif
if (system("rm -Rf /tmp/fish_uvars_test/")) err(L"rm failed"); if (system("rm -Rf /tmp/fish_uvars_test/")) err(L"rm failed");
} }