Revert "Adopt owning_lock and some cleanup of termsize storage in common.cpp"

Tests are failing on Travis but not locally

This reverts commit c5d9e7e391.
This commit is contained in:
ridiculousfish 2017-01-29 19:33:30 -08:00
parent 8aab725782
commit 0fefdb458f

View file

@ -61,9 +61,11 @@ static pid_t initial_pid = 0;
static pid_t initial_fg_process_group = -1; static pid_t initial_fg_process_group = -1;
/// This struct maintains the current state of the terminal size. It is updated on demand after /// This struct maintains the current state of the terminal size. It is updated on demand after
/// receiving a SIGWINCH. Use common_get_width()/common_get_height() to access. /// receiving a SIGWINCH. Do not touch this struct directly, it's managed with a rwlock. Use
owning_lock<struct winsize> termsize = {{USHRT_MAX, USHRT_MAX, USHRT_MAX, USHRT_MAX}}; /// common_get_width()/common_get_height().
static volatile sig_atomic_t termsize_valid = false; static pthread_mutex_t termsize_lock = PTHREAD_MUTEX_INITIALIZER;
static struct winsize termsize = {USHRT_MAX, USHRT_MAX, USHRT_MAX, USHRT_MAX};
static volatile bool termsize_valid = false;
static char *wcs2str_internal(const wchar_t *in, char *out); static char *wcs2str_internal(const wchar_t *in, char *out);
static void debug_shared(const wchar_t msg_level, const wcstring &msg); static void debug_shared(const wchar_t msg_level, const wcstring &msg);
@ -1358,12 +1360,10 @@ bool unescape_string(const wcstring &input, wcstring *output, unescape_flags_t e
/// COLUMNS or LINES variables are changed. This is also invoked when the shell regains control of /// COLUMNS or LINES variables are changed. This is also invoked when the shell regains control of
/// the tty since it is possible the terminal size changed while an external command was running. /// the tty since it is possible the terminal size changed while an external command was running.
void invalidate_termsize(bool invalidate_vars) { void invalidate_termsize(bool invalidate_vars) {
termsize_valid = false;
if (invalidate_vars) { if (invalidate_vars) {
auto lock_ts = termsize.acquire();
struct winsize &termsize = lock_ts.value;
termsize.ws_col = termsize.ws_row = USHRT_MAX; termsize.ws_col = termsize.ws_row = USHRT_MAX;
} }
termsize_valid = false;
} }
/// Handle SIGWINCH. This is also invoked when the shell regains control of the tty since it is /// Handle SIGWINCH. This is also invoked when the shell regains control of the tty since it is
@ -1372,7 +1372,7 @@ void common_handle_winch(int signal) {
// Don't run ioctl() here. Technically it's not safe to use in signals although in practice it // Don't run ioctl() here. Technically it's not safe to use in signals although in practice it
// is safe on every platform I've used. But we want to be conservative on such matters. // is safe on every platform I've used. But we want to be conservative on such matters.
UNUSED(signal); UNUSED(signal);
termsize_valid = false; invalidate_termsize(false);
} }
/// Validate the new terminal size. Fallback to the env vars if necessary. Ensure the values are /// Validate the new terminal size. Fallback to the env vars if necessary. Ensure the values are
@ -1413,11 +1413,11 @@ static void validate_new_termsize(struct winsize *new_termsize) {
} }
/// Export the new terminal size as env vars and to the kernel if possible. /// Export the new terminal size as env vars and to the kernel if possible.
static void export_new_termsize(const struct winsize &new_termsize) { static void export_new_termsize(struct winsize *new_termsize) {
wchar_t buf[64]; wchar_t buf[64];
swprintf(buf, 64, L"%d", (int)new_termsize.ws_col); swprintf(buf, 64, L"%d", (int)new_termsize->ws_col);
env_set(L"COLUMNS", buf, ENV_EXPORT | ENV_GLOBAL); env_set(L"COLUMNS", buf, ENV_EXPORT | ENV_GLOBAL);
swprintf(buf, 64, L"%d", (int)new_termsize.ws_row); swprintf(buf, 64, L"%d", (int)new_termsize->ws_row);
env_set(L"LINES", buf, ENV_EXPORT | ENV_GLOBAL); env_set(L"LINES", buf, ENV_EXPORT | ENV_GLOBAL);
#ifdef HAVE_WINSIZE #ifdef HAVE_WINSIZE
@ -1425,43 +1425,28 @@ static void export_new_termsize(const struct winsize &new_termsize) {
#endif #endif
} }
// Returns the current termsize by reference.
// Return true if it changed
static bool check_winsize_changes(struct winsize *out_result) {
auto lock_ts = termsize.acquire();
struct winsize &termsize = lock_ts.value;
bool changed = false;
if (! termsize_valid) {
struct winsize new_termsize = {0, 0, 0, 0};
#ifdef HAVE_WINSIZE
ioctl(STDOUT_FILENO, TIOCGWINSZ, &new_termsize);
#endif
if (new_termsize.ws_col != termsize.ws_col ||
new_termsize.ws_row != termsize.ws_row) {
validate_new_termsize(&new_termsize);
termsize.ws_col = new_termsize.ws_col;
termsize.ws_row = new_termsize.ws_row;
changed = true;
}
termsize_valid = true;
}
*out_result = termsize;
return changed;
}
/// Updates termsize as needed, and returns a copy of the winsize. /// Updates termsize as needed, and returns a copy of the winsize.
/// Note
struct winsize get_current_winsize() { struct winsize get_current_winsize() {
struct winsize new_termsize; scoped_lock guard(termsize_lock);
if (check_winsize_changes(&new_termsize)) {
// It changed! if (termsize_valid) return termsize;
// Note it's important that we don't hold the lock here,
// since we may be calling out to user code struct winsize new_termsize = {0, 0, 0, 0};
// TODO: we can't export except on the main thread #ifdef HAVE_WINSIZE
// need to make this thread safe! errno = 0;
export_new_termsize(new_termsize); if (ioctl(STDOUT_FILENO, TIOCGWINSZ, &new_termsize) != -1 &&
new_termsize.ws_col == termsize.ws_col && new_termsize.ws_row == termsize.ws_row) {
termsize_valid = true;
return termsize;
} }
return new_termsize; #endif
validate_new_termsize(&new_termsize);
export_new_termsize(&new_termsize);
termsize.ws_col = new_termsize.ws_col;
termsize.ws_row = new_termsize.ws_row;
termsize_valid = true;
return termsize;
} }
int common_get_width() { return get_current_winsize().ws_col; } int common_get_width() { return get_current_winsize().ws_col; }