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 29f0a674a8

Signed-off-by: Jim Warner <james.warner@comcast.net>
This commit is contained in:
Jim Warner 2021-10-28 00:00:00 -05:00 committed by Craig Small
parent 3e43e6273e
commit ad51fef1aa

View File

@ -286,6 +286,9 @@ static sem_t Semaphore_memory_beg, Semaphore_memory_end;
static pthread_t Thread_id_tasks; static pthread_t Thread_id_tasks;
static sem_t Semaphore_tasks_beg, Semaphore_tasks_end; static sem_t Semaphore_tasks_beg, Semaphore_tasks_end;
#endif #endif
#if defined THREADED_CPU || defined THREADED_MEM || defined THREADED_TSK
static pthread_t Thread_id_main;
#endif
/*###### Tiny useful routine(s) ########################################*/ /*###### 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 ... // there's lots of signal-unsafe stuff in the following ...
if (Frames_signal != BREAK_sig) { 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 #ifdef THREADED_CPU
pthread_cancel(Thread_id_cpus); pthread_cancel(Thread_id_cpus);
pthread_join(Thread_id_cpus, NULL); pthread_join(Thread_id_cpus, NULL);
@ -461,6 +469,9 @@ static void bye_bye (const char *str) {
procps_pids_unref(&Pids_ctx); procps_pids_unref(&Pids_ctx);
procps_stat_unref(&Stat_ctx); procps_stat_unref(&Stat_ctx);
procps_meminfo_unref(&Mem_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() | /* 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))); error_exit(fmtmk(N_fmt(LIB_errorpid_fmt),__LINE__, strerror(-rc)));
#if defined THREADED_CPU || defined THREADED_MEM || defined THREADED_TSK #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 /* 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). */ with everything blocked. therefore, signals go to the main thread (us). */
sigfillset(&sa.sa_mask); sigfillset(&sa.sa_mask);