summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLars Henriksen <LarsHenriksen@get2net.dk>2018-07-13 22:40:37 +0200
committerLukas Fleischer <lfleischer@calcurse.org>2018-07-28 14:06:15 +0200
commit5e5370864886c2e813b5335f6ef5cf5648f2ec02 (patch)
treebb5d4d67854e3525f79bf80165effced03ee1ce9
parent6a6c7117dee91c9fd11c2b30f34f70386f1fba21 (diff)
downloadcalcurse-5e5370864886c2e813b5335f6ef5cf5648f2ec02.tar.gz
calcurse-5e5370864886c2e813b5335f6ef5cf5648f2ec02.zip
Solve deadlock in notification bar
calcurse deadlocks when 1) an upcoming appointment is on display in the notification bar, 2) an external command (like help) is started, 3) the time for the upcoming appointment arrives, and 4) the external command is exited. The notification bar thread is stopped while the external command is running. Upon exit from the external command, the n-bar thread is restarted and calcurse locks. The cause is the way in which the main notification bar thread is stopped: static pthread_t notify_t_main; void notify_stop_main_thread(void) { if (notify_t_main) { pthread_cancel(notify_t_main); pthread_join(notify_t_main, NULL); } } Objects of type pthread_t are opaque and should not be accessed directly. Initially notify_t_main is an uninitialised static variable (0), but later it has a value, which may or may not be the thread id of the notification main thread. Note that the thread id after exit of a thread may become the thread id of a new thread. Thus the variable set when the thread is created, is invalid after exit of the thread. Specifically, the first time notify_stop_main_thread() is called (by notify_start_main_thread() before the thread is created) is harmless (because notify_t_main is 0). Calling notify_stop_main_thread() later may be either OK because the main thread is running, or harmless because no thread with id notify_t_main is running: the two functions will fail with return value ESRCH (no such process), or fatal because an unrelated thread with this thread id is running: it will be cancelled, and the join may or may not succeed depending on whether the thread is joinable or detached. The "unrelated thread" could be the next-appointment thread, notify_thread_app, launched by notify_check_next_app(). Always calling notify_stop_main_thread() before starting the main thread becomes fatal when notify_check_next_app() is called shortly before notify_start_main_thread(). This is the case in the scenario described. The next-app-thread is then running when notify_stop_main_thread() is called, and apparently it has the thread id of the old main thread (confirmed by logging the return values from pthread_cancel() and pthread_join(); the first succeeds while the second fails with EINVALID which means that the thread is not joinable). The next-app-thread will therefore exit without unlocking mutexes. Ensure that notify_t_main, in case the notify main thread is not running, has a value that it will never have when it is running. A possibility is the thread id of the main() calcurse process (returned by pthread_self()). Check for this condition in notify_stop_main_thread() and set notify_t_main when the thread is stopped. Similar changes have been introduced for the periodic save thread and the calendar date thread. Signed-off-by: Lars Henriksen <LarsHenriksen@get2net.dk> Signed-off-by: Lukas Fleischer <lfleischer@calcurse.org>
-rw-r--r--src/calcurse.h1
-rw-r--r--src/io.c6
-rw-r--r--src/notify.c13
-rw-r--r--src/ui-calendar.c12
-rw-r--r--src/vars.c12
5 files changed, 28 insertions, 16 deletions
diff --git a/src/calcurse.h b/src/calcurse.h
index 0d77e8f..05e27b0 100644
--- a/src/calcurse.h
+++ b/src/calcurse.h
@@ -1208,6 +1208,7 @@ extern struct pad apad;
extern struct nbar nbar;
extern struct dmon_conf dmon;
void vars_init(void);
+extern pthread_t notify_t_main, io_t_psave, ui_calendar_t_date;
/* wins.c */
extern struct window win[NBWINS];
diff --git a/src/io.c b/src/io.c
index d0c80c5..51adb42 100644
--- a/src/io.c
+++ b/src/io.c
@@ -1501,8 +1501,6 @@ void io_log_free(struct io_file *log)
mem_free(log);
}
-static pthread_t io_t_psave;
-
/* Thread used to periodically save data. */
static void *io_psave_thread(void *arg)
{
@@ -1524,7 +1522,8 @@ void io_start_psave_thread(void)
/* Stop periodic data saves. */
void io_stop_psave_thread(void)
{
- if (!io_t_psave)
+ /* Is the thread running? */
+ if (pthread_equal(io_t_psave, pthread_self()))
return;
/* Lock the mutex to avoid cancelling the thread during saving. */
@@ -1532,6 +1531,7 @@ void io_stop_psave_thread(void)
pthread_cancel(io_t_psave);
pthread_join(io_t_psave, NULL);
io_mutex_unlock();
+ io_t_psave = pthread_self();
}
/*
diff --git a/src/notify.c b/src/notify.c
index 7475611..5a44c0b 100644
--- a/src/notify.c
+++ b/src/notify.c
@@ -54,8 +54,6 @@ struct notify_vars {
static struct notify_vars notify;
static struct notify_app notify_app;
static pthread_attr_t detached_thread_attr;
-static pthread_t notify_t_main;
-static int notify_t_main_running;
/*
* Return the number of seconds before next appointment
@@ -194,12 +192,13 @@ void notify_free_app(void)
/* Stop the notify-bar main thread. */
void notify_stop_main_thread(void)
{
- if (!notify_t_main_running)
+ /* Is the thread running? */
+ if (pthread_equal(notify_t_main, pthread_self()))
return;
pthread_cancel(notify_t_main);
pthread_join(notify_t_main, NULL);
- notify_t_main_running = 0;
+ notify_t_main = pthread_self();
}
/*
@@ -555,12 +554,10 @@ int notify_same_recur_item(struct recur_apoint *i)
/* Launch the notify-bar main thread. */
void notify_start_main_thread(void)
{
- if (notify_t_main_running)
- return;
+ /* Avoid starting the notification bar thread twice. */
+ notify_stop_main_thread();
pthread_create(&notify_t_main, NULL, notify_main_thread, NULL);
- notify_t_main_running = 1;
-
notify_check_next_app(0);
}
diff --git a/src/ui-calendar.c b/src/ui-calendar.c
index b6d35b1..7d464dd 100644
--- a/src/ui-calendar.c
+++ b/src/ui-calendar.c
@@ -47,7 +47,6 @@
static struct date today, slctd_day;
static unsigned ui_calendar_view, week_begins_on_monday;
static pthread_mutex_t date_thread_mutex = PTHREAD_MUTEX_INITIALIZER;
-static pthread_t ui_calendar_t_date;
static void draw_monthly_view(struct scrollwin *, struct date *, unsigned);
static void draw_weekly_view(struct scrollwin *, struct date *, unsigned);
@@ -120,10 +119,13 @@ void ui_calendar_start_date_thread(void)
/* Stop the calendar date thread. */
void ui_calendar_stop_date_thread(void)
{
- if (ui_calendar_t_date) {
- pthread_cancel(ui_calendar_t_date);
- pthread_join(ui_calendar_t_date, NULL);
- }
+ /* Is the thread running? */
+ if (pthread_equal(ui_calendar_t_date, pthread_self()))
+ return;
+
+ pthread_cancel(ui_calendar_t_date);
+ pthread_join(ui_calendar_t_date, NULL);
+ ui_calendar_t_date = pthread_self();
}
/* Set static variable today to current date */
diff --git a/src/vars.c b/src/vars.c
index c2ec106..d3bc676 100644
--- a/src/vars.c
+++ b/src/vars.c
@@ -103,6 +103,15 @@ struct nbar nbar;
struct dmon_conf dmon;
/*
+ * Thread id variables for threads that never exit.
+ *
+ * Each variable either carries the identifier of the corresponding thread. If
+ * one of the threads is not running, the corresponding variable is assigned
+ * the identifier of the main thread instead.
+ */
+pthread_t notify_t_main, io_t_psave, ui_calendar_t_date;
+
+/*
* Variables init
*/
void vars_init(void)
@@ -166,4 +175,7 @@ void vars_init(void)
/* Start at the current date */
ui_calendar_init_slctd_day();
+
+ /* Threads not yet running. */
+ notify_t_main = io_t_psave = ui_calendar_t_date = pthread_self();
}