Hi Stuart, On Fri, Feb 01, 2019 at 11:39:09PM +0000, Stuart Henderson wrote: > +cc maintainer
Thanks, I should include Frederic in my initial email. > It feels like bugs on the calcurse side to me. > > First problem - program calls notify_start_main_thread to start the > notify thread. First thing this does is try to cancel any existing > notify thread by calling notify_stop_main_thread - on an OS where > pthread_t is just an identifier this would be a noop because there's a > "is not null" check, but on OpenBSD pthread_t is a struct so this is > never null, so it always tries to stop a (nonexistent) thread at first. > This doesn't cause a crash but it is wrong to try to stop a thread that > hasn't been started yet. > > Second problem - when you read help, first it stops the thread, then > it calls notify_start_main_thread to try to start it again, but again > (and this time I believe it will be the same on other OS too) it > tries to pthread_cancel/pthread_join the thread which it already > stopped. And this is where it goes boom on OpenBSD because pthread_cancel > tries to dereference a pointer that has already been freed. > > Not sure if this is a *good* diff but it does avoid the crash and > I don't think it's too terrible. > > Any comments/OKs? Your diff makes sense to me and it does make the crash go away. I've looked after your email to upstream repo and found they also fix the problem already in a slightly different way: https://git.calcurse.org/calcurse.git/commit/src/notify.c?id=30f411257ad3bc233184c08b846a2980a9c5d1f0 I've decided to redo OpenBSD ports diff with upstream's commit included. Comments, OKs? Index: Makefile =================================================================== RCS file: /cvs/ports/productivity/calcurse/Makefile,v retrieving revision 1.26 diff -u -p -u -r1.26 Makefile --- Makefile 19 Oct 2018 14:19:59 -0000 1.26 +++ Makefile 3 Feb 2019 19:27:05 -0000 @@ -3,7 +3,7 @@ COMMENT= text-based calendar and scheduling application DISTNAME= calcurse-4.3.0 -REVISION= 1 +REVISION= 2 EPOCH= 0 CATEGORIES= productivity Index: patches/patch-src_notify_c =================================================================== RCS file: patches/patch-src_notify_c diff -N patches/patch-src_notify_c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ patches/patch-src_notify_c 3 Feb 2019 19:27:05 -0000 @@ -0,0 +1,48 @@ +$OpenBSD$ + +Upstream git commit 30f411257ad3bc233184c08b846a2980a9c5d1f0: +- Do not stop already cancelled notification thread + +Index: src/notify.c +--- src/notify.c.orig ++++ src/notify.c +@@ -55,6 +55,7 @@ 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 +@@ -190,10 +191,12 @@ void notify_free_app(void) + /* Stop the notify-bar main thread. */ + void notify_stop_main_thread(void) + { +- if (notify_t_main) { +- pthread_cancel(notify_t_main); +- pthread_join(notify_t_main, NULL); +- } ++ if (!notify_t_main_running) ++ return; ++ ++ pthread_cancel(notify_t_main); ++ pthread_join(notify_t_main, NULL); ++ notify_t_main_running = 0; + } + + /* +@@ -549,10 +552,12 @@ int notify_same_recur_item(struct recur_apoint *i) + /* Launch the notify-bar main thread. */ + void notify_start_main_thread(void) + { +- /* Avoid starting the notification bar thread twice. */ +- notify_stop_main_thread(); ++ if (notify_t_main_running) ++ return; + + pthread_create(¬ify_t_main, NULL, notify_main_thread, NULL); ++ notify_t_main_running = 1; ++ + notify_check_next_app(0); + } + Regards, Mikolaj
