search

klange / toaruos

A completely-from-scratch hobby operating system: bootloader, kernel, drivers, C library, and userspace including a composited graphical UI, dynamic linker, syntax-highlighting text editor, network stack, etc.
https://toaruos.org/
University of Illinois/NCSA Open Source License
6.09k stars 477 forks source link

Closing plasma from terminal with ctrl+c doesn't work #185

Closed jkapi closed 3 years ago

jkapi commented 5 years ago

The window stays open, but you can't close or move it. It stays visible after logging out and is not in the list if you run ps

klange commented 5 years ago

The problem is related to thread cleanup. Unlike other graphical applications, plasma renders in a thread, and signal handling in ToaruOS is not the same as in a proper Unix-like, such that the SIGINT from ^C kills only the main thread, leaving the child thread running. The thread is still visible from ps -T, but an oversight in the design of our ps implementation hides it from just a bare ps call.

The most correct fix for this involves changes to signal handling. Ideally, when a multithreaded process is killed by a deadly signal (such as SIGINT), its threads should also be killed, which would correct this issue.

A short-term fix for this specific problem would be to add a SIGINT handler to plasma so that it can gracefully shut down its rendering thread on its own.

ps could also use some fixes so that a multithreaded program in which the main thread has died can still show up as if it were a top-level process, perhaps by ensuring that at least one thread for each PID is displayed.

jozefnagy1133 commented 5 years ago

I have fixed the error by adding one function (sigintHandler()) which, if called sets the should_exit variable to 1. Then, I have included signal.h to call signal(SIGINT, sigintHandler); right before while(!should_exit) loop. It takes some time for plasma to detect, but it successfully exits. Pull request should be there any time.