top: tweak some end-of-job logic when separate threads

The separate threads for background updates were added
to top in the commit shown below. At that time cleanup
logic was added to end-of-job processing to cancel any
active threads and destroy any semaphores then in use.

That seemed like simple good stewardship with an added
benefit of avoiding potential valgrind 'possibly lost'
warnings for 320 byte blocks. Those blocks represented
an initial stack allocation for each of three threads.

All of that worked perfectly if such code was executed
under the main thread. In other words, if the keyboard
or a signal directed to any thread was used to trigger
program end. However, it might not always be the case.

Each of those 'refresh' routines supporting a separate
thread interacts with a newlib interface. As a result,
each is required to check the library's return result.
Should some error be detected, 'error_exit' is called.
Now we've got big problems with that eoj cleanup code.

One can't 'pthread_cancel' and 'pthread_join' a thread
from withing that same thread. Thus, when an error was
returned by the library with threads active, top would
hang with no possibility of removal short of a reboot.

So, this commit only executes that cancel/join cleanup
code when we are running under the main thread. Should
program end be triggered by a library error under some
sibling thread, all such cleanup will now be bypassed.
In the latter case, we will rely on documentation that
says any thread calling exit(3) will end every thread.

[ now, the only time we'll see any valgrind warnings ]
[ is with a library error, which is the least likely ]
[ scenario for running valgrind & top to begin with. ]

[ besides, if the valgrind warnings became a problem ]
[ one could easily add a 'warning-suppression' file. ]

Reference(s):
. Sep 2021, top introduced threads
commit 29f0a674a85bfb92443c56f070956a7dd60bb5f7

Signed-off-by: Jim Warner <james.warner@comcast.net>

diff --git a/top/top.c b/top/top.c
index 97d12a5f1729b65a10e216638d582bf651df3101..86af85a2fcf172d8cc178b1d586c1ee8658579f2 100644 (file)
--- a/top/top.c
+++ b/top/top.c
@@ -286,6 +286,9 @@ static sem_t Semaphore_memory_beg, Semaphore_memory_end;
 static pthread_t Thread_id_tasks;
 static sem_t Semaphore_tasks_beg, Semaphore_tasks_end;
 #endif
+#if defined THREADED_CPU || defined THREADED_MEM || defined THREADED_TSK
+static pthread_t Thread_id_main;
+#endif
 \f
 /*######  Tiny useful routine(s)  ########################################*/
 
@@ -440,6 +443,11 @@ static void bye_bye (const char *str) {
 
    // there's lots of signal-unsafe stuff in the following ...
    if (Frames_signal != BREAK_sig) {
+#if defined THREADED_CPU || defined THREADED_MEM || defined THREADED_TSK
+      /* can not execute any cleanup from a sibling thread and
+         we will violate proper indentation to minimize impact */
+      if (pthread_equal(Thread_id_main, pthread_self())) {
+#endif
 #ifdef THREADED_CPU
       pthread_cancel(Thread_id_cpus);
       pthread_join(Thread_id_cpus, NULL);
@@ -461,6 +469,9 @@ static void bye_bye (const char *str) {
       procps_pids_unref(&Pids_ctx);
       procps_stat_unref(&Stat_ctx);
       procps_meminfo_unref(&Mem_ctx);
+#if defined THREADED_CPU || defined THREADED_MEM || defined THREADED_TSK
+      }
+#endif
    }
 
    /* we'll only have a 'str' if called by error_exit() |
@@ -3383,6 +3394,7 @@ static void before (char *me) {
       error_exit(fmtmk(N_fmt(LIB_errorpid_fmt),__LINE__, strerror(-rc)));
 
 #if defined THREADED_CPU || defined THREADED_MEM || defined THREADED_TSK
+   Thread_id_main = pthread_self();
    /* in case any of our threads have been enabled, they'll inherit this mask
       with everything blocked. therefore, signals go to the main thread (us). */
    sigfillset(&sa.sa_mask);