Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CFE-4401: Adjusted isreadable() to work without pthread_cancel when not available #5590

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion INSTALL
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ sudo apk add alpine-sdk lmdb-dev openssl-dev bison flex-dev acl-dev pcre2-dev au
* Termux (2020-04-24)

pkg install build-essential git autoconf automake bison flex liblmdb openssl pcre2 libacl libyaml
./autogen.sh --without-pam
LDFLAGS='-landroid-glob' ./autogen.sh --without-pam
make && make install

* OSX (2021-10-20)

Expand Down
8 changes: 8 additions & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,14 @@ CC="$PTHREAD_CC"
CFLAGS="$PTHREAD_CFLAGS $CFLAGS"
LIBS="$PTHREAD_LIBS $LIBS"

dnl ######################################################################
dnl libpromises/evalfunction.c isreadable() uses pthread_cancel so check
dnl if available and reverts to pthread_kill if not.
dnl This check must come after ACX_PTHREAD is called so that CC, CFLAGS,
dnl and LIBS are appropriately set for the local pthreads situation.
dnl ######################################################################
AC_CHECK_FUNCS([pthread_cancel])

dnl ######################################################################
dnl Whether to build extensions as builtin extensions or a separate
dnl plugin. The default is plugin.
Expand Down
2 changes: 2 additions & 0 deletions libpromises/dbm_test_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
included file COSL.txt.
*/

#ifndef __ANDROID__
#include <platform.h>
#include <stdlib.h> /* lrand48_r() */
#include <unistd.h> /* usleep(), syscall()/gettid() */
Expand Down Expand Up @@ -735,3 +736,4 @@ void RemoveFilament(DBFilament *filament)
free(filament);
CloseDB(db);
}
#endif /* not __ANDROID__ */
2 changes: 2 additions & 0 deletions libpromises/dbm_test_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
included file COSL.txt.
*/

#ifndef __ANDROID__
#ifndef CFENGINE_DBM_TEST_API_H
#define CFENGINE_DBM_TEST_API_H

Expand Down Expand Up @@ -55,3 +56,4 @@ DBFilament *FillUpDB(dbid db_id, int usage_pct);
void RemoveFilament(DBFilament *filament);

#endif /* CFENGINE_DBM_TEST_API_H */
#endif /* not __ANDROID__ */
26 changes: 26 additions & 0 deletions libpromises/evalfunction.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
#include <version_comparison.h>
#include <mutex.h> /* ThreadWait */
#include <glob_lib.h>
#include <signal_lib.h> /* MaskTerminationSignalsInThread */

#include <math_eval.h>

Expand Down Expand Up @@ -8513,10 +8514,29 @@ struct IsReadableThreadData
FnCallResult result;
};

#ifndef HAVE_PTHREAD_CANCEL
#define PTHREAD_CANCELED ((void *)-1)
static void ThreadSignalHandler(int signum)
{
pthread_exit(PTHREAD_CANCELED);
}
#endif

static void *IsReadableThreadRoutine(void *data)
{
assert(data != NULL);

#ifndef HAVE_PTHREAD_CANCEL
MaskTerminationSignalsInThread();
struct sigaction actions;
memset(&actions, 0, sizeof(actions));
sigemptyset(&actions.sa_mask);
actions.sa_flags = 0;
actions.sa_handler = ThreadSignalHandler;
sigaction(SIGHUP, &actions, NULL);
MaskTerminationSignalsInThread();
#endif

struct IsReadableThreadData *const thread_data = data;

// Give main thread time to call pthread_cond_timedwait(3)
Expand Down Expand Up @@ -8668,7 +8688,11 @@ static FnCallResult FnCallIsReadable(ARG_UNUSED EvalContext *const ctx,
"Read operation timed out, exceeded %ld seconds.", path,
timeout);

#ifdef HAVE_PTHREAD_CANCEL
ret = pthread_cancel(thread_data.thread);
#else
ret = pthread_kill(thread_data.thread, SIGUSR2);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@larsewi you had mentioned in my previous PR comment

Is it safe to join a "killed" thread? Will this call the signal handler installed by the agent. If so, is this fine? Also the man pthread_kill(3) says:

And so here I am using the downstream patch which sends a kill signal and adds a signal handler to which then calls pthread_exit which is the "other option" of how the joined thread could be terminated at the appropriate timeout moment.

After a canceled thread has terminated, a join with that thread
using pthread_join(3) obtains PTHREAD_CANCELED as the thread's
exit status. (Joining with a thread is the only way to know that
cancelation has completed.)

from https://www.man7.org/linux/man-pages/man3/pthread_cancel.3.html

#endif
if (ret != 0)
{
Log(LOG_LEVEL_ERR, "Failed to cancel thread");
Expand Down Expand Up @@ -8700,10 +8724,12 @@ static FnCallResult FnCallIsReadable(ARG_UNUSED EvalContext *const ctx,
return FnFailure();
}

#ifdef HAVE_PTHREAD_CANCEL
if (status == PTHREAD_CANCELED)
{
Log(LOG_LEVEL_DEBUG, "Thread was canceled");
}
#endif

return result;
}
Expand Down
Loading