From 979d76083d00eee5ba309873938d80429d4c325f Mon Sep 17 00:00:00 2001 From: Amphitryon <17864580+Amphitryon0@users.noreply.github.com> Date: Thu, 24 Nov 2022 15:15:39 -0600 Subject: [PATCH 01/16] Unify exit with user acknowledgement construct --- base.h | 18 ++++++++++++++---- calculator.c | 5 +---- config.c | 15 ++++----------- node_print.c | 4 +--- 4 files changed, 20 insertions(+), 22 deletions(-) diff --git a/base.h b/base.h index 16a1ffc..b0178bb 100644 --- a/base.h +++ b/base.h @@ -43,6 +43,19 @@ inline char awaitKeyFromUser() { return getchar(); } +/*------------------------------------------------------------------- +* Function: exitWithUserAcknowledgement +* +* Prints the given message, asks the user to press enter (not any +* key due to typical line-buffered input), and once the message has +* been acknowledged, exits with a non-zero status. +-------------------------------------------------------------------*/ +inline void exitWithUserAcknowledgement(const char *const message) { + printf("%sPress ENTER to exit.\n", message); + awaitKeyFromUser(); + exit(1); +} + /*------------------------------------------------------------------- * Function : checkMallocFailed * Inputs : void* p @@ -55,9 +68,6 @@ inline char awaitKeyFromUser() { -------------------------------------------------------------------*/ inline void checkMallocFailed(const void* const p) { if (ABSL_PREDICT_FALSE(p == NULL)) { - printf("Fatal error! Ran out of heap memory.\n"); - printf("Press enter to quit.\n"); - awaitKeyFromUser(); - exit(1); + exitWithUserAcknowledgement("Fatal error! Ran out of heap memory.\n"); } } diff --git a/calculator.c b/calculator.c index adaa4f9..5d8d279 100644 --- a/calculator.c +++ b/calculator.c @@ -1249,10 +1249,7 @@ void reallocateRecipes(BranchPath* newRoot, const enum Type_Sort* rearranged_rec } else { if (indexItem2 < 0) { - printf("Fatal error! indexItem2 was not set in a branch where it should have.\n"); - printf("Press enter to quit."); - awaitKeyFromUser(); - exit(1); + exitWithUserAcknowledgement("Fatal error! indexItem2 was not set in a branch where it should have.\n"); } // Two ingredients to navigate to, but order matters // Pick the larger-index number ingredient first, as it will reduce diff --git a/config.c b/config.c index 28bac69..e810276 100644 --- a/config.c +++ b/config.c @@ -23,11 +23,8 @@ void initConfig() { // This covers the case where it is missing or in the wrong folder. // If it's a permission problem, a user who created that problem should // be able to figure it out based on this as well. - printf("Could not read config.txt file. Please ensure that the file is " - "in the same folder as the program.\n"); - printf("Press ENTER to exit."); - awaitKeyFromUser(); - exit(1); + exitWithUserAcknowledgement("Could not read config.txt file. Please " + "ensure that the file is in the same folder as the program.\n"); } // Now, parse the file to use throughout the program's lifetime. config = malloc(sizeof(config_t)); @@ -36,9 +33,7 @@ void initConfig() { printf("Could not read settings in config.txt file. Please ensure that " "the file is formatted correctly. Error occurred at line %d.\n", config_error_line(config)); - printf("Press ENTER to exit."); - awaitKeyFromUser(); - exit(1); + exitWithUserAcknowledgement(""); } fclose(fp); } @@ -193,9 +188,7 @@ void validateConfig() { validateIntSettingMax("selectionMethod", Manual, &errors); if (errors) { - printf("Press ENTER to exit.\n"); - awaitKeyFromUser(); - exit(1); + exitWithUserAcknowledgement(""); } } diff --git a/node_print.c b/node_print.c index e462c16..4a1fab7 100644 --- a/node_print.c +++ b/node_print.c @@ -228,9 +228,7 @@ void printResults(const char *filename, const BranchPath *path) { FILE *fp = fopen(filename, "w"); if (fp == NULL) { printf("Could not locate %s... This is a bug.\n", filename); - printf("Press ENTER to exit.\n"); - awaitKeyFromUser(); - exit(1); + exitWithUserAcknowledgement(""); } // Write header information printFileHeader(fp); From d534061a4d84b0a27f9c15e596b1ac657a00671b Mon Sep 17 00:00:00 2001 From: Amphitryon <17864580+Amphitryon0@users.noreply.github.com> Date: Thu, 24 Nov 2022 15:21:00 -0600 Subject: [PATCH 02/16] Ensure greeting is first text printed Even when the config is invalid, the welcome message should be printed. --- start.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/start.c b/start.c index 71d5e19..e49eb80 100644 --- a/start.c +++ b/start.c @@ -147,6 +147,10 @@ uint64_t getSysRNG() { } int main() { + // Greeting message to user + printAsciiGreeting(); + printf("Welcome to Recipes@Home!\n"); + current_frame_record = UNSET_FRAME_RECORD; initConfig(); validateConfig(); @@ -162,9 +166,6 @@ int main() { init_level_cfg(); // set log level from config curl_global_init(CURL_GLOBAL_DEFAULT); // Initialize libcurl - // Greeting message to user - printAsciiGreeting(); - printf("Welcome to Recipes@Home!\n"); printf("Leave this program running as long as you want to search for new recipe orders.\n"); // Try to retrieve the record from the Blob server From cee34c72ecc8324c2b57ebb4a9887e1be5c49069 Mon Sep 17 00:00:00 2001 From: Amphitryon <17864580+Amphitryon0@users.noreply.github.com> Date: Thu, 24 Nov 2022 15:40:29 -0600 Subject: [PATCH 03/16] Delay instruction to leave the program open We shouldn't print this if some checks might result in immediately closing the program. --- start.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/start.c b/start.c index e49eb80..ab48274 100644 --- a/start.c +++ b/start.c @@ -166,8 +166,6 @@ int main() { init_level_cfg(); // set log level from config curl_global_init(CURL_GLOBAL_DEFAULT); // Initialize libcurl - printf("Leave this program running as long as you want to search for new recipe orders.\n"); - // Try to retrieve the record from the Blob server int blob_record = getFastestRecordOnBlob(); if (blob_record == 0) { @@ -182,6 +180,8 @@ int main() { if (checkGithubVer() == 1) return -1; + printf("Leave this program running as long as you want to search for new recipe orders.\n"); + // Verify that the results folder exists // If not, create the directory #if _IS_WINDOWS From 864932e7d8867cea1b7322c2ed3958d0639810c9 Mon Sep 17 00:00:00 2001 From: Amphitryon <17864580+Amphitryon0@users.noreply.github.com> Date: Fri, 21 Oct 2022 12:22:29 -0500 Subject: [PATCH 04/16] Use more verbose messages when loading local PB This gives more context for the "You did not set a new PB." message that normally appears at startup. Also, the explanation when we ignore PB.txt now appears even when it doesn't contain a number. --- start.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/start.c b/start.c index ab48274..798a00e 100644 --- a/start.c +++ b/start.c @@ -197,15 +197,15 @@ int main() { // The PB file may not have been created yet, so ignore the case where it is missing if (fp != NULL) { int PB_record; - if (fscanf(fp, "%d", &PB_record) == 1) { - if (PB_record < 1000) { - printf("The record stored in PB.txt can't be right... Ignoring.\n"); - } else { - current_frame_record = PB_record; - // Submit the user's fastest roadmap to the server for leaderboard purposes - // in case this was not submitted upon initial discovery - testRecord(current_frame_record); - } + if (fscanf(fp, "%d", &PB_record) == 1 && PB_record > 1000) { + printf("Your local PB is %d. Re-uploading just in case.\n", PB_record); + current_frame_record = PB_record; + // Submit the user's fastest roadmap to the server for leaderboard purposes + // in case this was not submitted upon initial discovery + testRecord(current_frame_record); + } + else { + printf("Your local PB is corrupted. Using %d instead.\n", current_frame_record); } fclose(fp); } From 1d295b74917712e4be25c5ffd0492e35488652c5 Mon Sep 17 00:00:00 2001 From: Amphitryon <17864580+Amphitryon0@users.noreply.github.com> Date: Thu, 24 Nov 2022 19:58:12 -0600 Subject: [PATCH 05/16] Print 1-indexed thread numbers --- calculator.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/calculator.c b/calculator.c index 5d8d279..f687e63 100644 --- a/calculator.c +++ b/calculator.c @@ -1626,7 +1626,7 @@ void logIterations(int ID, int stepIndex, const BranchPath * curNode, int iterat { char callString[30]; char iterationString[100]; - sprintf(callString, "Call %d", ID); + sprintf(callString, "Thread %d", ID+1); sprintf(iterationString, "%d steps currently taken, %d frames accumulated so far; %dk iterations", stepIndex, curNode->description.totalFramesTaken, iterationCount / 1000); recipeLog(level, "Calculator", "Info", callString, iterationString); @@ -1673,7 +1673,7 @@ Result calculateOrder(const int ID) { if (total_dives % branchInterval == 0) { char temp1[30]; char temp2[75]; - sprintf(temp1, "Thread %d", ID); + sprintf(temp1, "Thread %d", ID+1); sprintf(temp2, "Iteration Limit Reached: Backing Out and Searching New Branch %d", total_dives); recipeLog(3, "Calculator", "Info", temp1, temp2); } From d2504b725bec33dfee118e2eee320379dd206b4a Mon Sep 17 00:00:00 2001 From: Amphitryon <17864580+Amphitryon0@users.noreply.github.com> Date: Thu, 24 Nov 2022 20:12:02 -0600 Subject: [PATCH 06/16] Clarify new branch message This is intended to address some user confusion about what the number represents (some people thought it had to do with the contents of the branch and were worried about it being the same between threads) and whether the message is important or not (some people thought that numbers not printed had some checks being skipped and even worried that PBs would not be found on these branches). --- calculator.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/calculator.c b/calculator.c index f687e63..b6e5e4f 100644 --- a/calculator.c +++ b/calculator.c @@ -1668,15 +1668,14 @@ Result calculateOrder(const int ID) { curNode = initializeRoot(); root = curNode; // Necessary when printing results starting from root - total_dives++; - if (total_dives % branchInterval == 0) { char temp1[30]; char temp2[75]; sprintf(temp1, "Thread %d", ID+1); - sprintf(temp2, "Iteration Limit Reached: Backing Out and Searching New Branch %d", total_dives); + sprintf(temp2, "Searching new random branch (%d searched so far)", total_dives); recipeLog(3, "Calculator", "Info", temp1, temp2); } + total_dives++; // If the user is not exploring only one branch, reset when it is time // Start iteration loop From 5f92e3c3cacb5f79db4f82a512c1f37555f40c31 Mon Sep 17 00:00:00 2001 From: Amphitryon <17864580+Amphitryon0@users.noreply.github.com> Date: Thu, 24 Nov 2022 20:38:21 -0600 Subject: [PATCH 07/16] Print log statements all at once This prevents a fairly common problem where two log lines can end up interleaved. --- logger.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/logger.c b/logger.c index 76de98a..f182b0e 100644 --- a/logger.c +++ b/logger.c @@ -27,13 +27,10 @@ int recipeLog(int level, char *process, char *subProcess, char *activity, char * char data[200]; sprintf(date, "[%d-%02d-%02d %02d:%02d:%02d]", year, month, day, hours, mins, secs); sprintf(data, "[%s][%s][%s][%s]\n", process, subProcess, activity, entry); - printf("%s", date); - printf("%s", data); + printf("%s%s", date, data); FILE* fp = fopen("recipes.log", "a"); - fputs(date, fp); - fputs(data, fp); - + fprintf(fp, "%s%s", date, data); fclose(fp); } return 0; From 44004c70bd06d9c095052826f498192c3061efaa Mon Sep 17 00:00:00 2001 From: Amphitryon <17864580+Amphitryon0@users.noreply.github.com> Date: Fri, 25 Nov 2022 06:50:15 -0600 Subject: [PATCH 08/16] Ensure manual selection mode never restarts It was technically possible to perform 100,000 moves and then restart. More realistically, this change is mainly useful if someone lowers the default iteration limit. --- calculator.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/calculator.c b/calculator.c index b6e5e4f..74ffe16 100644 --- a/calculator.c +++ b/calculator.c @@ -1643,8 +1643,9 @@ void logIterations(int ID, int stepIndex, const BranchPath * curNode, int iterat -------------------------------------------------------------------*/ Result calculateOrder(const int ID) { enum SelectionMethod selectionMethod = getConfigInt("selectionMethod"); - // When performing in-order traversal, we never want to restart. - int freeRunning = selectionMethod == InOrder; + // When performing in-order or manual traversal, we never want to restart. + bool noRestart = selectionMethod == InOrder || selectionMethod == Manual; + bool freeRunning = false; int branchInterval = getConfigInt("branchLogInterval"); int total_dives = 0; BranchPath *curNode = NULL; // Deepest node at any particular point @@ -1679,7 +1680,7 @@ Result calculateOrder(const int ID) { // If the user is not exploring only one branch, reset when it is time // Start iteration loop - while (iterationCount < iterationLimit || freeRunning) { + while (iterationCount < iterationLimit || noRestart) { if (checkShutdownOnIndex(iterationCount)) { break; } @@ -1862,7 +1863,7 @@ Result calculateOrder(const int ID) { // Logging for progress display iterationCount++; if (iterationCount % (branchInterval * DEFAULT_ITERATION_LIMIT) == 0 - && (freeRunning || iterationLimit != DEFAULT_ITERATION_LIMIT)) { + && (noRestart || iterationLimit != DEFAULT_ITERATION_LIMIT)) { logIterations(ID, stepIndex, curNode, iterationCount, 3); } else if (iterationCount % 10000 == 0) { From 46c13139ddf46e0ab7c31ae1fbd4c3ee40030239 Mon Sep 17 00:00:00 2001 From: Amphitryon <17864580+Amphitryon0@users.noreply.github.com> Date: Fri, 25 Nov 2022 08:35:56 -0600 Subject: [PATCH 09/16] Disable new branch message when never restarting --- calculator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/calculator.c b/calculator.c index 74ffe16..4a01da1 100644 --- a/calculator.c +++ b/calculator.c @@ -1669,7 +1669,7 @@ Result calculateOrder(const int ID) { curNode = initializeRoot(); root = curNode; // Necessary when printing results starting from root - if (total_dives % branchInterval == 0) { + if (total_dives % branchInterval == 0 && !noRestart) { char temp1[30]; char temp2[75]; sprintf(temp1, "Thread %d", ID+1); From 3db180b4b6144e7758d8cb7a8ddffb9e8131313d Mon Sep 17 00:00:00 2001 From: Amphitryon <17864580+Amphitryon0@users.noreply.github.com> Date: Fri, 25 Nov 2022 12:41:17 -0600 Subject: [PATCH 10/16] Simplify iterations log message The extra information about steps and frames was extremely confusing to users and, as a pure debugging feature, was inappropriate at log level 3, if not even at log level 6. --- calculator.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/calculator.c b/calculator.c index 4a01da1..9aa2456 100644 --- a/calculator.c +++ b/calculator.c @@ -1619,16 +1619,15 @@ Inventory getSortedInventory(Inventory inventory, enum Action sort) { /*------------------------------------------------------------------- * Function : logIterations * - * Print out information to the user about how deep we are and - * the frame cost at this point after a certain number of iterations. + * Print out information to the user about how far we are in the + * current branch. -------------------------------------------------------------------*/ -void logIterations(int ID, int stepIndex, const BranchPath * curNode, int iterationCount, int level) +void logIterations(int ID, int iterationCount, int level) { char callString[30]; - char iterationString[100]; + char iterationString[50]; sprintf(callString, "Thread %d", ID+1); - sprintf(iterationString, "%d steps currently taken, %d frames accumulated so far; %dk iterations", - stepIndex, curNode->description.totalFramesTaken, iterationCount / 1000); + sprintf(iterationString, "Explored %dk iterations", iterationCount / 1000); recipeLog(level, "Calculator", "Info", callString, iterationString); } @@ -1661,7 +1660,6 @@ Result calculateOrder(const int ID) { if (askedToShutdown()) { break; } - int stepIndex = 0; int iterationCount = 0; int iterationLimit = DEFAULT_ITERATION_LIMIT; @@ -1732,7 +1730,6 @@ Result calculateOrder(const int ID) { curNode = curNode->prev; freeAndShiftLegalMove(curNode, 0); curNode->next = NULL; - stepIndex--; continue; } // End condition not met. Check if this current level has something in the event queue @@ -1791,7 +1788,6 @@ Result calculateOrder(const int ID) { curNode = curNode->prev; freeAndShiftLegalMove(curNode, 0); curNode->next = NULL; - stepIndex--; continue; } @@ -1829,7 +1825,6 @@ Result calculateOrder(const int ID) { curNode->next = curNode->legalMoves[0]; curNode = curNode->next; - stepIndex++; } else { @@ -1846,7 +1841,6 @@ Result calculateOrder(const int ID) { curNode = curNode->prev; freeAndShiftLegalMove(curNode, 0); curNode->next = NULL; - stepIndex--; continue; } @@ -1858,16 +1852,15 @@ Result calculateOrder(const int ID) { // Once the list is generated, choose the top-most (quickest) path and iterate downward curNode->next = curNode->legalMoves[0]; curNode = curNode->legalMoves[0]; - stepIndex++; // Logging for progress display iterationCount++; if (iterationCount % (branchInterval * DEFAULT_ITERATION_LIMIT) == 0 && (noRestart || iterationLimit != DEFAULT_ITERATION_LIMIT)) { - logIterations(ID, stepIndex, curNode, iterationCount, 3); + logIterations(ID, iterationCount, 3); } else if (iterationCount % 10000 == 0) { - logIterations(ID, stepIndex, curNode, iterationCount, 6); + logIterations(ID, iterationCount, 6); } } } From 1071d176817a5ccb21a1ea4932232a3301391cf0 Mon Sep 17 00:00:00 2001 From: Amphitryon <17864580+Amphitryon0@users.noreply.github.com> Date: Sat, 26 Nov 2022 09:50:43 -0600 Subject: [PATCH 11/16] Add input validation in manual move selection mode --- calculator.c | 33 +++++++++++++++++++++++++++++++-- calculator.h | 1 + 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/calculator.c b/calculator.c index 9aa2456..9a1ee04 100644 --- a/calculator.c +++ b/calculator.c @@ -1616,6 +1616,36 @@ Inventory getSortedInventory(Inventory inventory, enum Action sort) { return inventory; } +/*------------------------------------------------------------------- + * Function: takeNumericInputInRange + * + * Take a number from stdin, reprompting if it is misformatted or + * not between minimum and maximum, inclusive. + -------------------------------------------------------------------*/ +int takeNumericInputInRange(int minimum, int maximum) { + char userResponse[40]; + // Keep asking forever until a valid input is given. + while (1) { + if (fgets(userResponse, sizeof(userResponse), stdin) != NULL) { + // If the response was too long, go ahead and clear anything else + // that was typed on that line. + if (strchr(userResponse, '\n') == NULL) { + int c; + while ((c = getchar()) != '\n' && c != EOF); + } + else { + int value; + // Parse the response as an integer and check that it is in the + // required range. + if (sscanf(userResponse, "%d", &value) == 1 + && value >= minimum && value <= maximum) + return value; + } + } + printf("Invalid value. Enter a number %d to %d. ", minimum, maximum); + } +} + /*------------------------------------------------------------------- * Function : logIterations * @@ -1803,8 +1833,7 @@ Result calculateOrder(const int ID) { fprintf(fp, "%d - Run freely\n", curNode->numLegalMoves); printf("Which move would you like to perform? "); - int moveToExplore; - ABSL_ATTRIBUTE_UNUSED int ignored = scanf("%d", &moveToExplore); // For now, we are going to blindly assume it was written. + int moveToExplore = takeNumericInputInRange(0, curNode->numLegalMoves); fprintf(fp, "\n"); if (moveToExplore == curNode->numLegalMoves) { diff --git a/calculator.h b/calculator.h index 495de90..c771ecb 100644 --- a/calculator.h +++ b/calculator.h @@ -109,6 +109,7 @@ BranchPath* initializeRoot(); // Other void periodicGithubCheck(); +int takeNumericInputInRange(int minimum, int maximum); Result calculateOrder(const int ID); void writePersonalBest(Result *result); void shutdownCalculator(); From 22071c4e08d3daf4108c8d87b9296eeb6fa909d8 Mon Sep 17 00:00:00 2001 From: Amphitryon <17864580+Amphitryon0@users.noreply.github.com> Date: Sun, 27 Nov 2022 07:09:26 -0600 Subject: [PATCH 12/16] Fix worker count logic This was accidentally checking for the Random move selection method instead of Manual. --- start.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/start.c b/start.c index 798a00e..cc7a8ef 100644 --- a/start.c +++ b/start.c @@ -160,7 +160,7 @@ int main() { // we should likewise use only 1, or else multiple threads will be // interacting with the user at once. enum SelectionMethod method = getConfigInt("selectionMethod"); - int workerCount = (method == InOrder || method == Random) ? 1 + int workerCount = (method == InOrder || method == Manual) ? 1 : getConfigInt("workerCount"); init_level_cfg(); // set log level from config From 122adfd403aea6a76d27a37eb17e4cdbc474bd2d Mon Sep 17 00:00:00 2001 From: Amphitryon <17864580+Amphitryon0@users.noreply.github.com> Date: Sun, 27 Nov 2022 18:56:46 -0600 Subject: [PATCH 13/16] Move shutdown handlers to proper source file --- base.h | 1 + shutdown.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ shutdown.h | 2 +- start.c | 51 --------------------------------------------------- 4 files changed, 54 insertions(+), 52 deletions(-) diff --git a/base.h b/base.h index b0178bb..5e3bded 100644 --- a/base.h +++ b/base.h @@ -2,6 +2,7 @@ #include #include +#include #include "absl/base/port.h" #if defined(_MSC_FULL_VER) || defined(_MSC_VER) || defined(__MINGW32__) diff --git a/shutdown.c b/shutdown.c index 2cfb510..58214fa 100644 --- a/shutdown.c +++ b/shutdown.c @@ -1,4 +1,10 @@ #include "shutdown.h" +#include +#include "base.h" + +#if _IS_WINDOWS +#include +#endif bool _askedToShutdownVar = false; @@ -8,3 +14,49 @@ bool requestShutdown() { return oldVal; } +int numTimesExitRequest = 0; +#define NUM_TIMES_EXITED_BEFORE_HARD_QUIT 3 + +void countAndSetShutdown(bool isSignal) { + // On Windows, it is undefined behavior trying to use stdio.h functions in a signal handler (which this is called from). + // So for now, these messages are stifled on Windows. This may be revisited at a later point. + if (++numTimesExitRequest >= NUM_TIMES_EXITED_BEFORE_HARD_QUIT) { + if (!_IS_WINDOWS || !isSignal) { + printf("\nExit reqested %d times; shutting down now.\n", NUM_TIMES_EXITED_BEFORE_HARD_QUIT); + } + exit(1); + } + else { + requestShutdown(); + if (!_IS_WINDOWS || !isSignal) { + printf("\nExit requested, finishing up work. Should shutdown soon (CTRL-C %d times total to force exit)\n", NUM_TIMES_EXITED_BEFORE_HARD_QUIT); + } + } +} + +void handleTermSignal(int signal) { + countAndSetShutdown(true); +} + +#if _IS_WINDOWS +BOOL WINAPI windowsCtrlCHandler(DWORD fdwCtrlType) { + switch (fdwCtrlType) { + case CTRL_C_EVENT: ABSL_FALLTHROUGH_INTENDED; + case CTRL_CLOSE_EVENT: + countAndSetShutdown(false); + return TRUE; + default: + return FALSE; + } +} +#endif + +void setSignalHandlers() { + signal(SIGTERM, handleTermSignal); + signal(SIGINT, handleTermSignal); +#if _IS_WINDOWS + if (!SetConsoleCtrlHandler(windowsCtrlCHandler, TRUE)) { + printf("Unable to set CTRL-C handler. CTRL-C may cause unclean shutdown.\n"); + } +#endif +} diff --git a/shutdown.h b/shutdown.h index 0c5ac4b..e592fc9 100644 --- a/shutdown.h +++ b/shutdown.h @@ -6,7 +6,7 @@ extern bool _askedToShutdownVar; -bool requestShutdown(); +void setSignalHandlers(); ABSL_ATTRIBUTE_ALWAYS_INLINE inline bool askedToShutdown() { return ABSL_PREDICT_FALSE(_askedToShutdownVar); diff --git a/start.c b/start.c index cc7a8ef..48eddec 100644 --- a/start.c +++ b/start.c @@ -2,7 +2,6 @@ #include #include -#include #include #include #include @@ -17,10 +16,6 @@ #include "shutdown.h" #include "types.h" -#if _IS_WINDOWS -#include -#endif - #define UNSET_FRAME_RECORD 9999 int current_frame_record; @@ -43,52 +38,6 @@ void setLocalRecord(int frames) { current_frame_record = frames; } -int numTimesExitRequest = 0; -#define NUM_TIMES_EXITED_BEFORE_HARD_QUIT 3 - -void countAndSetShutdown(bool isSignal) { - // On Windows, it is undefined behavior trying to use stdio.h functions in a signal handler (which this is called from). - // So for now, these messages are stifled on Windows. This may be revisited at a later point. - if (++numTimesExitRequest >= NUM_TIMES_EXITED_BEFORE_HARD_QUIT) { - if (!_IS_WINDOWS || !isSignal) { - printf("\nExit reqested %d times; shutting down now.\n", NUM_TIMES_EXITED_BEFORE_HARD_QUIT); - } - exit(1); - } else { - requestShutdown(); - if (!_IS_WINDOWS || !isSignal) { - printf("\nExit requested, finishing up work. Should shutdown soon (CTRL-C %d times total to force exit)\n", NUM_TIMES_EXITED_BEFORE_HARD_QUIT); - } - } -} - -void handleTermSignal(int signal) { - countAndSetShutdown(true); -} - -#if _IS_WINDOWS -BOOL WINAPI windowsCtrlCHandler(DWORD fdwCtrlType) { - switch (fdwCtrlType) { - case CTRL_C_EVENT: ABSL_FALLTHROUGH_INTENDED; - case CTRL_CLOSE_EVENT: - countAndSetShutdown(false); - return TRUE; - default: - return FALSE; - } -} -#endif - -void setSignalHandlers() { - signal(SIGTERM, handleTermSignal); - signal(SIGINT, handleTermSignal); -#if _IS_WINDOWS - if (!SetConsoleCtrlHandler(windowsCtrlCHandler, TRUE)) { - printf("Unable to set CTRL-C handler. CTRL-C may cause unclean shutdown.\n"); - } -#endif -} - void printAsciiGreeting() { FILE *fp = fopen("img.txt", "r"); From 866e74d86ec9b15d9080565f6db3a9c884b867d3 Mon Sep 17 00:00:00 2001 From: Amphitryon <17864580+Amphitryon0@users.noreply.github.com> Date: Thu, 20 Oct 2022 16:16:32 -0500 Subject: [PATCH 14/16] Improve system-specific clean shutdown support Windows does not generate SIGTERM when closing a program, and it actually doesn't support SIGINT either; it may work, but correct behavior is not guaranteed. --- shutdown.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/shutdown.c b/shutdown.c index 58214fa..956c9e8 100644 --- a/shutdown.c +++ b/shutdown.c @@ -1,9 +1,10 @@ #include "shutdown.h" -#include #include "base.h" #if _IS_WINDOWS #include +#else +#include #endif bool _askedToShutdownVar = false; @@ -34,10 +35,6 @@ void countAndSetShutdown(bool isSignal) { } } -void handleTermSignal(int signal) { - countAndSetShutdown(true); -} - #if _IS_WINDOWS BOOL WINAPI windowsCtrlCHandler(DWORD fdwCtrlType) { switch (fdwCtrlType) { @@ -49,14 +46,19 @@ BOOL WINAPI windowsCtrlCHandler(DWORD fdwCtrlType) { return FALSE; } } +#else +void handleTermSignal(int signal) { + countAndSetShutdown(true); +} #endif void setSignalHandlers() { - signal(SIGTERM, handleTermSignal); - signal(SIGINT, handleTermSignal); #if _IS_WINDOWS if (!SetConsoleCtrlHandler(windowsCtrlCHandler, TRUE)) { printf("Unable to set CTRL-C handler. CTRL-C may cause unclean shutdown.\n"); } +#else + signal(SIGTERM, handleTermSignal); + signal(SIGINT, handleTermSignal); #endif } From 16c3d7be31a88c6992b2739d9a39b8aca67eb8cd Mon Sep 17 00:00:00 2001 From: Amphitryon <17864580+Amphitryon0@users.noreply.github.com> Date: Thu, 20 Oct 2022 16:16:32 -0500 Subject: [PATCH 15/16] Handle all Windows signals correctly Using Ctrl+Break to shut down and closing the window both resulted in instant termination. --- shutdown.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/shutdown.c b/shutdown.c index 956c9e8..e8abfda 100644 --- a/shutdown.c +++ b/shutdown.c @@ -38,10 +38,24 @@ void countAndSetShutdown(bool isSignal) { #if _IS_WINDOWS BOOL WINAPI windowsCtrlCHandler(DWORD fdwCtrlType) { switch (fdwCtrlType) { + // For these events, returning TRUE is sufficient to prevent shutdown, so + // we can simply inform the threads to shut down. case CTRL_C_EVENT: ABSL_FALLTHROUGH_INTENDED; + case CTRL_BREAK_EVENT: + countAndSetShutdown(false); + return TRUE; + // Returning TRUE for a CTRL_CLOSE_EVENT results in instant termination, so + // we have to wait until the threads are done. case CTRL_CLOSE_EVENT: countAndSetShutdown(false); + // Note that this does not cause closing to take 5 seconds. The program + // will actually shut down when main returns. 5 seconds is just the max + // time we could delay shutdown. + Sleep(5000); return TRUE; + // CTRL_LOGOFF_EVENT and CTRL_SHUTDOWN_EVENT are not sent to interactive + // applications, only to services. Thus, there is no behavior to add in + // here. default: return FALSE; } From cdbf56d56a0dc6e95ab50c5e992fbc52da34220c Mon Sep 17 00:00:00 2001 From: Amphitryon <17864580+Amphitryon0@users.noreply.github.com> Date: Fri, 21 Oct 2022 12:43:40 -0500 Subject: [PATCH 16/16] Only allow uploading the best record available This check prevents now obsoleted records from being submitted, as the thread with the actual record either already has returned or will at some point. --- start.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/start.c b/start.c index 48eddec..11979c4 100644 --- a/start.c +++ b/start.c @@ -182,8 +182,9 @@ int main() { } Result result = calculateOrder(ID); - // result might store -1 frames for errors that might be recoverable - if (result.frames > -1) { + // Multiple threads might have discovered a record, so make sure + // result is still the best we have. + if (result.frames == getLocalRecord()) { testRecord(result.frames); } }