From b5927353e05feaa80fb762aedcee4ce8298927a4 Mon Sep 17 00:00:00 2001 From: Pietro Gagliardi Date: Tue, 28 May 2019 21:54:13 -0400 Subject: [PATCH] Started work to test that functions dislike being called before uiInit() or on a different thread; better to do it now than later. --- common/errors.c | 2 + common/init.c | 14 ++++ common/uipriv.h | 5 ++ darwin/main.m | 10 +++ test/allcalls.h | 3 + test/meson.build | 1 + test/noinitwrongthread.c | 174 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 209 insertions(+) create mode 100644 test/allcalls.h create mode 100644 test/noinitwrongthread.c diff --git a/common/errors.c b/common/errors.c index b6c6a273..216b3e60 100644 --- a/common/errors.c +++ b/common/errors.c @@ -30,6 +30,8 @@ void uiprivInternalError(const char *fmt, ...) } static const char *messages[uiprivNumProgrammerErrors] = { + [uiprivProgrammerErrorNotInitialized] = "attempt to call %s() before uiInit()", + [uiprivProgrammerErrorWrongThread] = "attempt to call %s() on a thread other than the GUI thread", [uiprivProgrammerErrorWrongStructSize] = "wrong size %" uiprivSizetPrintf " for %s", [uiprivProgrammerErrorIndexOutOfRange] = "index %d out of range in %s()", [uiprivProgrammerErrorNullPointer] = "invalid null pointer for %s passed into %s()", diff --git a/common/init.c b/common/init.c index 83015402..6ea9ea5c 100644 --- a/common/init.c +++ b/common/init.c @@ -77,3 +77,17 @@ int uiprivInitReturnErrorf(uiInitError *err, const char *msg, ...) va_end(ap); return 0; } + +bool uiprivCheckInitializedAndThreadImpl(const char *func) +{ + // While it does seem risky to not lock this, if this changes during the execution of this function it means only that it was changed from a different thread, and since it can only change from false to true, an error will be reported anyway. + if (!initialized) { + uiprivProgrammerError(uiprivProgrammerErrorNotInitialized, func); + return false; + } + if (!uiprivSysCheckThread()) { + uiprivProgrammerError(uiprivProgrammerErrorWrongThread, func); + return false; + } + return true; +} diff --git a/common/uipriv.h b/common/uipriv.h index da57f7dc..befbbeac 100644 --- a/common/uipriv.h +++ b/common/uipriv.h @@ -24,6 +24,9 @@ extern const char **uiprivSysInitErrors(void); extern int uiprivSysInit(void *options, uiInitError *err); extern int uiprivInitReturnError(uiInitError *err, const char *msg); extern int uiprivInitReturnErrorf(uiInitError *err, const char *msg, ...); +extern bool uiprivCheckInitializedAndThreadImpl(const char *func); +#define uiprivCheckInitializedAndThread() uiprivCheckInitializedAndThreadImpl(uiprivFunc) +extern bool uiprivSysCheckThread(void); // alloc.c extern void *uiprivAlloc(size_t n, const char *what); @@ -58,6 +61,8 @@ extern void uiprivArrayQsort(uiprivArray *arr, int (*compare)(const void *, cons // errors.c extern void uiprivInternalError(const char *fmt, ...); enum { + uiprivProgrammerErrorNotInitialized, // arguments: uiprivFunc + uiprivProgrammerErrorWrongThread, // arguments: uiprivFunc uiprivProgrammerErrorWrongStructSize, // arguments: size_t badSize, const char *structName uiprivProgrammerErrorIndexOutOfRange, // arguments: int badIndex, uiprivFunc uiprivProgrammerErrorNullPointer, // arguments: const char *paramDesc, uiprivFunc diff --git a/darwin/main.m b/darwin/main.m index 05a3c063..72dd704a 100644 --- a/darwin/main.m +++ b/darwin/main.m @@ -49,8 +49,12 @@ const char **uiprivSysInitErrors(void) return initErrors; } +static pthread_t mainThread; + int uiprivSysInit(void *options, uiInitError *err) { + int lockerr; + uiprivApp = [uiprivApplication sharedApplication]; if (![NSApp isKindOfClass:[uiprivApplication class]]) return uiprivInitReturnError(err, errNSAppAlreadyInitialized); @@ -62,6 +66,7 @@ int uiprivSysInit(void *options, uiInitError *err) uiprivAppDelegate = [uiprivApplicationDelegate new]; [uiprivApp setDelegate:uiprivAppDelegate]; + mainThread = pthread_self(); return 1; } @@ -99,6 +104,11 @@ void uiQueueMain(void (*f)(void *data), void *data) dispatch_async_f(dispatch_get_main_queue(), data, f); } +bool uiprivSysCheckThread(void) +{ + return pthread_equal(pthread_self(), mainThread); +} + // Debugger() was deprecated in macOS 10.8 (as part of the larger CarbonCore deprecation), but they did not provide a replacement. // Though some people say inline asm, I'd rather make this work automatically everywhere. // Others say raise(SIGTRAP) and others still say __builtin_trap(), but I can't confirm these do what I want (some sources, including documentation, claim they either cause a core dump or send a SIGILL instead). diff --git a/test/allcalls.h b/test/allcalls.h new file mode 100644 index 00000000..36966859 --- /dev/null +++ b/test/allcalls.h @@ -0,0 +1,3 @@ +// 28 may 2019 + +// This file should NOT have include guards as it is intended to be included more than once; see noinitwrongthread.c for details. diff --git a/test/meson.build b/test/meson.build index b18d7122..ff816ddd 100644 --- a/test/meson.build +++ b/test/meson.build @@ -4,6 +4,7 @@ libui_test_sources = [ 'events.c', 'initmain.c', 'main.c', + 'noinitwrongthread.c', ] if libui_OS == 'windows' diff --git a/test/noinitwrongthread.c b/test/noinitwrongthread.c new file mode 100644 index 00000000..0b5db00b --- /dev/null +++ b/test/noinitwrongthread.c @@ -0,0 +1,174 @@ +// 28 may 2019 +#include "test.h" + +struct errorCase { + const char *name; + bool caught; + char *prefixGot; + bool internalGot; + char *msgGot; + const char *msgWant; + struct errorCase *next; +}; + +static struct errorCase *current = NULL; +static bool memoryExhausted = false; + +static void catalogProgrammerError(const char *prefix, const char *msg, const char *suffix, bool internal) +{ + current->caught = true; + if (strstr(prefix, "programmer error") == NULL) { + current->prefixGot = (char *) malloc((strlen(prefix) + 1) * sizeof (char)); + if (current->prefixGot == NULL) { + memoryExhausted = true; + return; + } + strcpy(current->prefixGot, prefix); + } + current->internalGot = internal; + if (strstr(msg, current->msgWant) == NULL) { + current->msgGot = (char *) malloc((strlen(msg) + 1) * sizeof (char)); + if (current->msgGot == NULL) { + memoryExhausted = true; + return; + } + strcpy(current->msgGot, msg); + } +} + +static struct errorCase *newCase(void) +{ + struct errorCase *p; + + p = (struct errorCase *) malloc(sizeof (struct errorCase)); + if (p == NULL) { + memoryExhausted = true; + return NULL; + } + memset(p, 0, sizeof (struct errorCase)); + return p; +} + +static void freeCases(struct errorCase *first) +{ + struct errorCase *p, *next; + + p = first; + while (p != NULL) { + if (p->prefixGot != NULL) + free(p->prefixGot); + if (p->msgGot != NULL) + free(p->msgGot); + next = p->next; + free(p); + p = next; + } +} + +static void reportCases(testingT *t, struct errorCase *p) +{ + while (p != NULL) { + testingTLogf(t, "*** %s", t->name); + if (!p->caught) { + testingTErrorf(t, "%s did not throw a programmer error; should have", p->name); + p = p->next; + continue; + } + if (p->prefixGot != NULL) + testingTErrorf(t, "%s prefix string doesn't contain \"programmer error\": %s", p->name, p->prefixGot); + if (p->internalGot) + testingTErrorf(t, "%s error is marked internal; should not have been", p->name); + if (p->msgGot != NULL) + diff_2str(t, p->name, "message doesn't contain expected substring", + "%s", p->msgGot, p->msgWant); + p = p->next; + } +} + +#define allcallsCase(f, ...) { \ + current = newCase(); \ + if (memoryExhausted) \ + return first; \ + current->name = #f "()"; \ + current->msgWant = "attempt to call " #f "() " allcallsMsgSuffix; \ + f(__VA_LIST__); \ + if (first == NULL) \ + first = current; \ + if (last != NULL) \ + last->next = current; \ + last = current; \ + if (memoryExhausted) \ + return first; \ +} + +static struct errorCase *runCasesBeforeInit(void) +{ + struct errorCase *first = NULL; + struct errorCase *last = NULL; + +#define allcallsMsgSuffix "before uiInit()" + allcallsCase(uiQueueMain, NULL, NULL); +#include "allcalls.h" +#undef allcallsMsgSuffix + return first; +} + +testingTestInSet(beforeTests, FunctionsFailBeforeInit) +{ + struct errorCase *cases; + + memoryExhausted = false; + uiprivTestHookReportProgrammerError(catalogProgrammerError); + cases = runCasesBeforeInit(); + uiprivTestHookReportProgrammerError(NULL); + if (memoryExhausted) { + freeCases(cases); + testingTFatalf(t, "memory exhausted running tests"); + } + reportCases(t, cases); + freeCases(cases); +} + +static struct errorCase *runCasesWrongThread(void) +{ + struct errorCase *first = NULL; + struct errorCase *last = NULL; + +#define allcallsMsgSuffix "on a thread other than the GUI thread" +#include "allcalls.h" +#undef allcallsMsgSuffix + return first; +} + +static void wrongThreadThreadProc(void *data) +{ + struct errorCase **pCases = (struct errorCase **) data; + + uiprivTestHookReportProgrammerError(catalogProgrammerError); + *pCases = runCasesWrongThread(); + uiprivTestHookReportProgrammerError(NULL); + // do this now in case something gets allocated before we return to the main thread + if (memoryExhausted) { + freeCases(*pCases); + *pCases = NULL; + } +} + +testingTest(FunctionsFailOnWrongThread) +{ + struct errorCase *cases; + threadThread *thread; + threadSysError err; + + memoryExhausted = false; + err = threadNewThread(wrongThreadThreadProc, &cases, &thread); + if (err != 0) + testingTFatalf(t, "error creating thread: " threadSysErrorFmt, threadSysErrorFmtArg(err)); + err = threadThreadWaitAndFree(thread); + if (err != 0) + testingTFatalf(t, "error waiting for thread to finish: " threadSysErrorFmt, threadSysErrorFmtArg(err)); + if (memoryExhausted) + testingTFatalf(t, "memory exhausted running tests"); + reportCases(t, cases); + freeCases(cases); +}