macOS: Fix a regression in the previous release that caused kitty to fail to run after an unclean shutdown/crash

When porting the single instance code to C I forgot to port the lock
file usage.

Fixes #7846
This commit is contained in:
Kovid Goyal
2024-09-15 10:05:18 +05:30
parent 5a77ea64d0
commit d1bdbddbfc
5 changed files with 80 additions and 21 deletions

View File

@@ -87,6 +87,8 @@ Detailed list of changes
- Fix a regression that broke writing to the clipboard using the OSC 5522 protocol (:iss:`7858`) - Fix a regression that broke writing to the clipboard using the OSC 5522 protocol (:iss:`7858`)
- macOS: Fix a regression in the previous release that caused kitty to fail to run after an unclean shutdown/crash (:iss:`7846`)
0.36.2 [2024-09-06] 0.36.2 [2024-09-06]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

View File

@@ -217,6 +217,7 @@ run_embedded(RunData *run_data) {
fail: fail:
PyConfig_Clear(&config); PyConfig_Clear(&config);
if (PyStatus_IsExit(status)) return status.exitcode; if (PyStatus_IsExit(status)) return status.exitcode;
single_instance_main(-1, NULL, NULL);
Py_ExitStatusException(status); Py_ExitStatusException(status);
} }
@@ -462,5 +463,6 @@ int main(int argc, char *argv[], char* envp[]) {
if (num < 0 || num >= PATH_MAX) { fprintf(stderr, "Failed to create path to kitty lib\n"); return 1; } if (num < 0 || num >= PATH_MAX) { fprintf(stderr, "Failed to create path to kitty lib\n"); return 1; }
RunData run_data = {.exe = exe, .exe_dir = exe_dir, .lib_dir = lib, .argc = argc, .argv = argv, .lc_ctype = lc_ctype}; RunData run_data = {.exe = exe, .exe_dir = exe_dir, .lib_dir = lib, .argc = argc, .argv = argv, .lc_ctype = lc_ctype};
ret = run_embedded(&run_data); ret = run_embedded(&run_data);
single_instance_main(-1, NULL, NULL);
return ret; return ret;
} }

View File

@@ -29,7 +29,7 @@
#define NO_SINGLE_BYTE_CHARSETS #define NO_SINGLE_BYTE_CHARSETS
#include "../charsets.c" #include "../charsets.c"
#define fail_on_errno(msg) { perror(msg); exit(1); } #define fail_on_errno(msg) { perror(msg); do_exit(1); }
void void
log_error(const char *fmt, ...) { log_error(const char *fmt, ...) {
@@ -39,6 +39,37 @@ log_error(const char *fmt, ...) {
va_end(ar); va_end(ar);
} }
typedef struct cleanup_data {
int fd1, fd2;
bool close_fd1, close_fd2;
char path1[sizeof(struct sockaddr_un) + 16], path2[sizeof(struct sockaddr_un) + 16];
} cleanup_data;
struct {
cleanup_data si, notify;
} cleanup_entries = {0};
static void
do_cleanup(cleanup_data *d) {
if (d->path1[0]) unlink(d->path1);
if (d->path2[0]) unlink(d->path2);
if (d->close_fd1) safe_close(d->fd1, __FILE__, __LINE__);
if (d->close_fd2) safe_close(d->fd2, __FILE__, __LINE__);
}
static void
cleanup(void) {
do_cleanup(&cleanup_entries.notify);
do_cleanup(&cleanup_entries.si);
}
static void
do_exit(int code) {
cleanup();
exit(code);
}
#ifndef __APPLE__ #ifndef __APPLE__
static bool static bool
is_ok_tmpdir(const char *x) { is_ok_tmpdir(const char *x) {
@@ -79,11 +110,10 @@ get_socket_dir(char *output, size_t output_capacity) {
} }
static void static void
set_single_instance_socket(int fd, const char *socket_path) { set_single_instance_socket(int fd) {
if (listen(fd, 5) != 0) fail_on_errno("Failed to listen on single instance socket"); if (listen(fd, 5) != 0) fail_on_errno("Failed to listen on single instance socket");
char buf[256]; char buf[256];
if (socket_path && socket_path[0]) snprintf(buf, sizeof(buf), "%d:%s", fd, socket_path); snprintf(buf, sizeof(buf), "%d", fd);
else snprintf(buf, sizeof(buf), "%d", fd);
setenv("KITTY_SI_DATA", buf, 1); setenv("KITTY_SI_DATA", buf, 1);
} }
@@ -159,8 +189,9 @@ read_till_eof(FILE *f, membuf *m) {
fclose(f); fclose(f);
} }
static bool static bool
bind_unix_socket(int s, const char *basename, struct sockaddr_un *addr) { bind_unix_socket(int s, const char *basename, struct sockaddr_un *addr, cleanup_data *cleanup) {
addr->sun_family = AF_UNIX; addr->sun_family = AF_UNIX;
const size_t blen = strlen(basename); const size_t blen = strlen(basename);
// First try abstract socket // First try abstract socket
@@ -173,9 +204,28 @@ bind_unix_socket(int s, const char *basename, struct sockaddr_un *addr) {
const size_t dlen = strlen(addr->sun_path); const size_t dlen = strlen(addr->sun_path);
if (snprintf(addr->sun_path + dlen, sizeof(addr->sun_path) - dlen, "/%s", basename) < blen + 1) { if (snprintf(addr->sun_path + dlen, sizeof(addr->sun_path) - dlen, "/%s", basename) < blen + 1) {
fprintf(stderr, "Socket directory has path too long for single instance socket file %s\n", addr->sun_path); fprintf(stderr, "Socket directory has path too long for single instance socket file %s\n", addr->sun_path);
exit(1); do_exit(1);
}
// First lock the socket file using a separate lock file
char lock_file_path[sizeof(addr->sun_path) + 16];
snprintf(lock_file_path, sizeof(lock_file_path), "%s.lock", addr->sun_path);
int fd = safe_open(lock_file_path, O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC, S_IRUSR | S_IWUSR);
if (fd == -1) return false;
cleanup->close_fd2 = true; cleanup->fd2 = fd;
snprintf(cleanup->path2, sizeof(cleanup->path2), "%s", lock_file_path);
if (safe_lockf(fd, F_TLOCK, 0) != 0) {
int saved_errno = errno;
safe_close(fd, __FILE__, __LINE__);
errno = saved_errno;
if (errno == EAGAIN || errno == EACCES) errno = EADDRINUSE; // client
return false;
}
// First unlink the socket file and then try to bind it.
if (unlink(addr->sun_path) != 0 && errno != ENOENT) return false;
if (safe_bind(s, (struct sockaddr*)addr, sizeof(*addr)) > -1) {
snprintf(cleanup->path1, sizeof(cleanup->path1), "%s", addr->sun_path);
return true;
} }
if (safe_bind(s, (struct sockaddr*)addr, sizeof(*addr)) > -1) return true;
return false; return false;
} }
@@ -193,6 +243,7 @@ extern char **environ;
static void static void
talk_to_instance(int s, struct sockaddr_un *server_addr, int argc, char *argv[], const CLIOptions *opts) { talk_to_instance(int s, struct sockaddr_un *server_addr, int argc, char *argv[], const CLIOptions *opts) {
cleanup_entries.si.path2[0] = 0; cleanup_entries.si.path1[0] = 0;
membuf session_data = {0}; membuf session_data = {0};
if (opts->session && opts->session[0]) { if (opts->session && opts->session[0]) {
if (strcmp(opts->session, "none") == 0) { if (strcmp(opts->session, "none") == 0) {
@@ -235,10 +286,11 @@ talk_to_instance(int s, struct sockaddr_un *server_addr, int argc, char *argv[],
int notify_socket = -1; int notify_socket = -1;
if (opts->wait_for_single_instance_window_close) { if (opts->wait_for_single_instance_window_close) {
notify_socket = create_unix_socket(); notify_socket = create_unix_socket();
cleanup_entries.notify.fd1 = notify_socket; cleanup_entries.notify.close_fd1 = true;
struct sockaddr_un server_addr; struct sockaddr_un server_addr;
char addr[128]; char addr[128];
snprintf(addr, sizeof(addr), "kitty-os-window-close-notify-%d-%d", getpid(), geteuid()); snprintf(addr, sizeof(addr), "kitty-os-window-close-notify-%d-%d", getpid(), geteuid());
if (!bind_unix_socket(notify_socket, addr, &server_addr)) fail_on_errno("Failed to bind notification socket"); if (!bind_unix_socket(notify_socket, addr, &server_addr, &cleanup_entries.notify)) fail_on_errno("Failed to bind notification socket");
size_t len = strlen(server_addr.sun_path); size_t len = strlen(server_addr.sun_path);
if (len == 0) len = 1 + strlen(server_addr.sun_path +1); if (len == 0) len = 1 + strlen(server_addr.sun_path +1);
if (listen(notify_socket, 5) != 0) fail_on_errno("Failed to listen on notify socket"); if (listen(notify_socket, 5) != 0) fail_on_errno("Failed to listen on notify socket");
@@ -282,15 +334,17 @@ talk_to_instance(int s, struct sockaddr_un *server_addr, int argc, char *argv[],
void void
single_instance_main(int argc, char *argv[], const CLIOptions *opts) { single_instance_main(int argc, char *argv[], const CLIOptions *opts) {
if (argc == -1) { cleanup(); return; }
struct sockaddr_un server_addr; struct sockaddr_un server_addr;
char addr_buf[sizeof(server_addr.sun_path)-1]; char addr_buf[sizeof(server_addr.sun_path)-1];
if (opts->instance_group) snprintf(addr_buf, sizeof(addr_buf), "kitty-ipc-%d-%s", geteuid(), opts->instance_group); if (opts->instance_group) snprintf(addr_buf, sizeof(addr_buf), "kitty-ipc-%d-%s", geteuid(), opts->instance_group);
else snprintf(addr_buf, sizeof(addr_buf), "kitty-ipc-%d", geteuid()); else snprintf(addr_buf, sizeof(addr_buf), "kitty-ipc-%d", geteuid());
int s = create_unix_socket(); int s = create_unix_socket();
if (!bind_unix_socket(s, addr_buf, &server_addr)) { cleanup_entries.si.fd1 = s; cleanup_entries.si.close_fd1 = true;
if (errno == EADDRINUSE) { talk_to_instance(s, &server_addr, argc, argv, opts); exit(0); } if (!bind_unix_socket(s, addr_buf, &server_addr, &cleanup_entries.si)) {
if (errno == EADDRINUSE) { talk_to_instance(s, &server_addr, argc, argv, opts); do_exit(0); }
else fail_on_errno("Failed to bind single instance socket"); else fail_on_errno("Failed to bind single instance socket");
} else set_single_instance_socket(s, server_addr.sun_path); } else set_single_instance_socket(s);
} }

View File

@@ -485,16 +485,7 @@ def _main() -> None:
if cli_opts.single_instance: if cli_opts.single_instance:
si_data = os.environ.pop('KITTY_SI_DATA', '') si_data = os.environ.pop('KITTY_SI_DATA', '')
if si_data: if si_data:
import atexit talk_fd = int(si_data)
fdnum, sep, socket_path = si_data.partition(':')
talk_fd = int(fdnum)
def cleanup_si() -> None:
with suppress(OSError):
os.close(talk_fd)
with suppress(OSError):
if sep and socket_path:
os.unlink(socket_path)
atexit.register(cleanup_si)
bad_lines: list[BadLine] = [] bad_lines: list[BadLine] = []
opts = create_opts(cli_opts, accumulate_bad_lines=bad_lines) opts = create_opts(cli_opts, accumulate_bad_lines=bad_lines)
setup_environment(opts, cli_opts) setup_environment(opts, cli_opts)

View File

@@ -10,6 +10,16 @@
#include <sys/mman.h> #include <sys/mman.h>
#include <sys/socket.h> #include <sys/socket.h>
#include <stdlib.h> #include <stdlib.h>
#include <unistd.h>
static inline int
safe_lockf(int fd, int function, off_t size) {
while (true) {
int ret = lockf(fd, function, size);
if (ret != 0 && (errno == EINTR)) continue;
return ret;
}
}
static inline int static inline int
safe_connect(int socket_fd, struct sockaddr *addr, socklen_t addrlen) { safe_connect(int socket_fd, struct sockaddr *addr, socklen_t addrlen) {