From 392f576d074c45c2e5a224efbdddc12809b674c1 Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Tue, 14 Jul 2020 20:18:04 +0530 Subject: [PATCH] Instrument safe_close() to make it easy to see where an fd is closed from --- kitty/child-monitor.c | 14 +++++++------- kitty/child.c | 16 ++++++++-------- kitty/data-types.c | 2 +- kitty/data-types.h | 7 ++++++- kitty/graphics.c | 10 +++++----- kitty/loop-utils.c | 6 +++--- 6 files changed, 30 insertions(+), 25 deletions(-) diff --git a/kitty/child-monitor.c b/kitty/child-monitor.c index 58468908e..cbb77454d 100644 --- a/kitty/child-monitor.c +++ b/kitty/child-monitor.c @@ -766,7 +766,7 @@ thread_write(void *x) { if (pos < data->sz) { log_error("Failed to write all data to STDIN of child process with error: %s", strerror(errno)); } - safe_close(data->fd); + safe_close(data->fd, __FILE__, __LINE__); free_twd(data); return 0; } @@ -783,7 +783,7 @@ cm_thread_write(PyObject UNUSED *self, PyObject *args) { data->fd = fd; memcpy(data->buf, buf, data->sz); int ret = pthread_create(&thread, NULL, thread_write, data); - if (ret != 0) { safe_close(fd); free_twd(data); return PyErr_SetFromErrno(PyExc_OSError); } + if (ret != 0) { safe_close(fd, __FILE__, __LINE__); free_twd(data); return PyErr_SetFromErrno(PyExc_OSError); } pthread_detach(thread); Py_RETURN_NONE; } @@ -1017,7 +1017,7 @@ hangup(pid_t pid) { static inline void cleanup_child(ssize_t i) { - safe_close(children[i].fd); + safe_close(children[i].fd, __FILE__, __LINE__); hangup(children[i].pid); } @@ -1308,7 +1308,7 @@ typedef struct { static TalkData talk_data = {0}; typedef struct pollfd PollFD; #define PEER_LIMIT 256 -#define nuke_socket(s) { shutdown(s, SHUT_RDWR); safe_close(s); } +#define nuke_socket(s) { shutdown(s, SHUT_RDWR); safe_close(s, __FILE__, __LINE__); } static inline bool accept_peer(int listen_fd, bool shutting_down) { @@ -1466,7 +1466,7 @@ prune_finished_writes(void) { PeerWriteData *wd = talk_data.writes + i; if (wd->finished) { remove_poll_fd(wd->fd); - shutdown(wd->fd, SHUT_WR); safe_close(wd->fd); + shutdown(wd->fd, SHUT_WR); safe_close(wd->fd, __FILE__, __LINE__); free(wd->data); ssize_t num_to_right = talk_data.num_writes - 1 - i; if (num_to_right > 0) memmove(talk_data.writes + i, talk_data.writes + i + 1, num_to_right * sizeof(PeerWriteData)); @@ -1586,8 +1586,8 @@ add_peer_writer(int fd, const char* msg, size_t msg_sz) { static void send_response(int fd, const char *msg, size_t msg_sz) { - if (msg == NULL) { shutdown(fd, SHUT_WR); safe_close(fd); return; } - if (!add_peer_writer(fd, msg, msg_sz)) { shutdown(fd, SHUT_WR); safe_close(fd); } + if (msg == NULL) { shutdown(fd, SHUT_WR); safe_close(fd, __FILE__, __LINE__); return; } + if (!add_peer_writer(fd, msg, msg_sz)) { shutdown(fd, SHUT_WR); safe_close(fd, __FILE__, __LINE__); } else wakeup_talk_loop(false); } diff --git a/kitty/child.c b/kitty/child.c index 54edc6908..b539bbca0 100644 --- a/kitty/child.c +++ b/kitty/child.c @@ -99,28 +99,28 @@ spawn(PyObject *self UNUSED, PyObject *args) { // On BSD open() does not establish the controlling terminal if (ioctl(tfd, TIOCSCTTY, 0) == -1) exit_on_err("Failed to set controlling terminal with TIOCSCTTY"); #endif - safe_close(tfd); + safe_close(tfd, __FILE__, __LINE__); // Redirect stdin/stdout/stderr to the pty if (dup2(slave, 1) == -1) exit_on_err("dup2() failed for fd number 1"); if (dup2(slave, 2) == -1) exit_on_err("dup2() failed for fd number 2"); if (stdin_read_fd > -1) { if (dup2(stdin_read_fd, 0) == -1) exit_on_err("dup2() failed for fd number 0"); - safe_close(stdin_read_fd); - safe_close(stdin_write_fd); + safe_close(stdin_read_fd, __FILE__, __LINE__); + safe_close(stdin_write_fd, __FILE__, __LINE__); } else { if (dup2(slave, 0) == -1) exit_on_err("dup2() failed for fd number 0"); } - safe_close(slave); - safe_close(master); + safe_close(slave, __FILE__, __LINE__); + safe_close(master, __FILE__, __LINE__); // Wait for READY_SIGNAL which indicates kitty has setup the screen object - safe_close(ready_write_fd); + safe_close(ready_write_fd, __FILE__, __LINE__); wait_for_terminal_ready(ready_read_fd); - safe_close(ready_read_fd); + safe_close(ready_read_fd, __FILE__, __LINE__); // Close any extra fds inherited from parent - for (int c = 3; c < 201; c++) safe_close(c); + for (int c = 3; c < 201; c++) safe_close(c, __FILE__, __LINE__); environ = env; // for some reason SIGPIPE is set to SIG_IGN, so reset it, needed by bash, diff --git a/kitty/data-types.c b/kitty/data-types.c index 5b21cc711..acc2bea2c 100644 --- a/kitty/data-types.c +++ b/kitty/data-types.c @@ -142,7 +142,7 @@ close_tty(PyObject *self UNUSED, PyObject *args) { TTY_ARGS tcsetattr(fd, TCSAFLUSH, termios_p); // deliberately ignore failure free(termios_p); - safe_close(fd); + safe_close(fd, __FILE__, __LINE__); Py_RETURN_NONE; } diff --git a/kitty/data-types.h b/kitty/data-types.h index ce4ce1161..154f32917 100644 --- a/kitty/data-types.h +++ b/kitty/data-types.h @@ -312,5 +312,10 @@ void play_canberra_sound(const char *which_sound, const char *event_id); SPRITE_MAP_HANDLE alloc_sprite_map(unsigned int, unsigned int); SPRITE_MAP_HANDLE free_sprite_map(SPRITE_MAP_HANDLE); -static inline void safe_close(int fd) { while(close(fd) != 0 && errno == EINTR); } +static inline void safe_close(int fd, const char* file UNUSED, const int line UNUSED) { +#if 0 + printf("Closing fd: %d from file: %s line: %d\n", fd, file, line); +#endif + while(close(fd) != 0 && errno == EINTR); +} void log_event(const char *format, ...) __attribute__((format(printf, 1, 2))); diff --git a/kitty/graphics.c b/kitty/graphics.c index 31077a02d..809d868b8 100644 --- a/kitty/graphics.c +++ b/kitty/graphics.c @@ -404,7 +404,7 @@ handle_add_command(GraphicsManager *self, const GraphicsCommand *g, const uint8_ else fd = open(fname, O_CLOEXEC | O_RDONLY); if (fd == -1) ABRT(EBADF, "Failed to open file %s for graphics transmission with error: [%d] %s", fname, errno, strerror(errno)); img->data_loaded = mmap_img_file(self, img, fd, g->data_sz, g->data_offset); - safe_close(fd); + safe_close(fd, __FILE__, __LINE__); if (tt == 't') { if (global_state.boss) { call_boss(safe_delete_temp_file, "s", fname); } else unlink(fname); @@ -891,12 +891,12 @@ W(shm_write) { int fd = shm_open(name, O_CREAT | O_RDWR, S_IRUSR | S_IWUSR); if (fd == -1) { PyErr_SetFromErrnoWithFilename(PyExc_OSError, name); return NULL; } int ret = ftruncate(fd, sz); - if (ret != 0) { safe_close(fd); PyErr_SetFromErrnoWithFilename(PyExc_OSError, name); return NULL; } + if (ret != 0) { safe_close(fd, __FILE__, __LINE__); PyErr_SetFromErrnoWithFilename(PyExc_OSError, name); return NULL; } void *addr = mmap(0, sz, PROT_WRITE, MAP_SHARED, fd, 0); - if (addr == MAP_FAILED) { safe_close(fd); PyErr_SetFromErrnoWithFilename(PyExc_OSError, name); return NULL; } + if (addr == MAP_FAILED) { safe_close(fd, __FILE__, __LINE__); PyErr_SetFromErrnoWithFilename(PyExc_OSError, name); return NULL; } memcpy(addr, data, sz); - if (munmap(addr, sz) != 0) { safe_close(fd); PyErr_SetFromErrnoWithFilename(PyExc_OSError, name); return NULL; } - safe_close(fd); + if (munmap(addr, sz) != 0) { safe_close(fd, __FILE__, __LINE__); PyErr_SetFromErrnoWithFilename(PyExc_OSError, name); return NULL; } + safe_close(fd, __FILE__, __LINE__); Py_RETURN_NONE; } diff --git a/kitty/loop-utils.c b/kitty/loop-utils.c index e5c2307e0..b64e81518 100644 --- a/kitty/loop-utils.c +++ b/kitty/loop-utils.c @@ -48,7 +48,7 @@ handle_signal(int sig_num) { void free_loop_data(LoopData *ld) { -#define CLOSE(which, idx) if (ld->which[idx] > -1) safe_close(ld->which[idx]); ld->which[idx] = -1; +#define CLOSE(which, idx) if (ld->which[idx] > -1) safe_close(ld->which[idx], __FILE__, __LINE__); ld->which[idx] = -1; #ifndef HAS_EVENT_FD CLOSE(wakeup_fds, 0); CLOSE(wakeup_fds, 1); #endif @@ -58,7 +58,7 @@ free_loop_data(LoopData *ld) { #undef CLOSE if (ld->signal_read_fd) { #ifdef HAS_SIGNAL_FD - safe_close(ld->signal_read_fd); + safe_close(ld->signal_read_fd, __FILE__, __LINE__); SIGNAL_SET sigprocmask(SIG_UNBLOCK, &signals, NULL); #else @@ -69,7 +69,7 @@ free_loop_data(LoopData *ld) { signal(SIGCHLD, SIG_DFL); } #ifdef HAS_EVENT_FD - safe_close(ld->wakeup_read_fd); + safe_close(ld->wakeup_read_fd, __FILE__, __LINE__); #endif ld->signal_read_fd = -1; ld->wakeup_read_fd = -1; }