From bf029b46768a2ab4447b71e17a02ae34984fd6da Mon Sep 17 00:00:00 2001 From: abraunegg Date: Sun, 28 Apr 2024 09:05:30 +1000 Subject: [PATCH] Update ^C Handling during upload|download operations * Update ^C Handling during upload|download operations --- src/curlEngine.d | 41 +++++++++++++++++++---------------------- src/log.d | 1 + src/main.d | 18 ++++++------------ src/onedrive.d | 21 ++++++++------------- src/sync.d | 32 ++++++++++++-------------------- 5 files changed, 46 insertions(+), 67 deletions(-) diff --git a/src/curlEngine.d b/src/curlEngine.d index 11712712..afc1bddd 100644 --- a/src/curlEngine.d +++ b/src/curlEngine.d @@ -161,13 +161,14 @@ class CurlResponse { class CurlEngine { - __gshared CurlEngine[] curlEnginePool; - + // Shared pool of CurlEngine instances accessible across all threads + __gshared CurlEngine[] curlEnginePool; // __gshared is used to declare a variable that is shared across all threads + HTTP http; + File uploadFile; + CurlResponse response; bool keepAlive; ulong dnsTimeout; - CurlResponse response; - File uploadFile; string internalThreadId; this() { @@ -176,31 +177,27 @@ class CurlEngine { internalThreadId = generateAlphanumericString(); } + // The destructor should only clean up resources owned directly by this instance ~this() { - // The destructor should only clean up resources owned directly by this instance - addLogEntry("CurlEngine DESTRUCTOR CALLED", ["debug"]); - addLogEntry("CurlEngine DESTRUCTOR CALLED"); - - // Avoid modifying or destroying shared/static resources here + // Is the file still open? if (uploadFile.isOpen()) { uploadFile.close(); } - - // Cleanup class memory usage - object.destroy(uploadFile); // Destroy, however we cant set to null - object.destroy(response); // Destroy, then set to null - response = null; - internalThreadId = null; + + // Is 'response' cleared? + if (response !is null) { + object.destroy(response); // Destroy, then set to null + response = null; + } // Is the actual http instance is stopped? if (!http.isStopped) { - // instance was not stopped .. need to stop it - addLogEntry("CurlEngine DESTRUCTOR HTTP INSTANCE WAS NOT STOPPED - STOPPING", ["debug"]); + // HTTP instance was not stopped .. need to stop it http.shutdown(); + object.destroy(http); // Destroy, however we cant set to null } - object.destroy(http); // Destroy, however we cant set to null } - + static CurlEngine getCurlInstance() { synchronized (CurlEngine.classinfo) { // What is the current pool size @@ -232,6 +229,7 @@ class CurlEngine { synchronized (CurlEngine.classinfo) { // What is the current pool size addLogEntry("CURL ENGINES TO RELEASE: " ~ to!string(curlEnginePool.length), ["debug"]); + addLogEntry("CURL ENGINES TO RELEASE: " ~ to!string(curlEnginePool.length)); // Safely iterate and clean up each CurlEngine instance foreach (curlEngineInstance; curlEnginePool) { @@ -253,12 +251,11 @@ class CurlEngine { // Destroy all curl instances static void destroyAllCurlInstances() { + addLogEntry("DESTROY ALL CURL ENGINES", ["debug"]); + addLogEntry("DESTROY ALL CURL ENGINES"); // Release all 'curl' instances releaseAllCurlInstances(); - // Destroy curlEnginePool, set to null - object.destroy(curlEnginePool); - curlEnginePool = null; } // We are releasing a curl instance back to the pool diff --git a/src/log.d b/src/log.d index 0648f952..904fc22a 100644 --- a/src/log.d +++ b/src/log.d @@ -53,6 +53,7 @@ class LogBuffer { flushThread.start(); } + // The destructor should only clean up resources owned directly by this instance ~this() { object.destroy(bufferLock); object.destroy(condReady); diff --git a/src/main.d b/src/main.d index 5de315c7..77535fae 100644 --- a/src/main.d +++ b/src/main.d @@ -1367,13 +1367,8 @@ extern(C) nothrow @nogc @system void exitHandler(int value) { assumeNoGC ( () { addLogEntry("\nReceived termination signal, initiating cleanup"); // Wait for all parallel jobs that depend on the database to complete - addLogEntry("Waiting for any existing upload|download process to complete"); - taskPool.finish(true); - - // Force kill any running threads - //addLogEntry("Forcing any active thread to exit"); - //taskPool.finish(false); + syncEngineInstance.shutdown(); // Perform the shutdown process performSynchronisedExitProcess("exitHandler"); @@ -1403,17 +1398,16 @@ void performSynchronisedExitProcess(string scopeCaller = null) { shutdownOneDriveWebhook(); // Shutdown the client side filtering objects shutdownSelectiveSync(); + // Destroy all 'curl' instances + destroyCurlInstances(); // Shutdown the sync engine shutdownSyncEngine(); // Shutdown any local filesystem monitoring shutdownFilesystemMonitor(); // Shutdown the database shutdownDatabase(); - // Destroy all 'curl' instances - destroyCurlInstances(); // Shutdown the application configuration objects shutdownAppConfig(); - } catch (Exception e) { addLogEntry("Error during performStandardExitProcess: " ~ e.toString(), ["error"]); } @@ -1449,7 +1443,7 @@ void shutdownFilesystemMonitor() { void shutdownSelectiveSync() { if (selectiveSync !is null) { addLogEntry("Shutdown Client Side Filtering instance", ["debug"]); - selectiveSync.shutdown(); + selectiveSync.shutdown(); object.destroy(selectiveSync); selectiveSync = null; } @@ -1458,7 +1452,7 @@ void shutdownSelectiveSync() { void shutdownSyncEngine() { if (syncEngineInstance !is null) { addLogEntry("Shutdown Sync Engine instance", ["debug"]); - //syncEngineInstance.shutdown(); - potentially need this and also check for a ~this() for class cleanup + syncEngineInstance.shutdown(); // Make sure any running thread completes first object.destroy(syncEngineInstance); syncEngineInstance = null; } @@ -1480,7 +1474,7 @@ void shutdownAppConfig() { // We were running with --dry-run , clean up the applicable database cleanupDryRunDatabaseFiles(runtimeDatabaseFile); } - object.destroy(appConfig); + object.destroy(appConfig); appConfig = null; } } diff --git a/src/onedrive.d b/src/onedrive.d index c1bea315..dce2dafd 100644 --- a/src/onedrive.d +++ b/src/onedrive.d @@ -111,17 +111,19 @@ class OneDriveApi { siteSearchUrl = appConfig.globalGraphEndpoint ~ "/v1.0/sites?search"; siteDriveUrl = appConfig.globalGraphEndpoint ~ "/v1.0/sites/"; - // Subscriptions subscriptionUrl = appConfig.globalGraphEndpoint ~ "/v1.0/subscriptions"; } + // The destructor should only clean up resources owned directly by this instance ~this() { - // We cant destroy 'appConfig' here as this leads to a segfault - object.destroy(curlEngine); - object.destroy(response); - curlEngine = null; - response = null; + if (curlEngine !is null) { + curlEngine = null; + } + + if (response !is null) { + response = null; + } } // Initialise the OneDrive API class @@ -353,12 +355,6 @@ class OneDriveApi { return authorised; } - // Reinitialise the OneDrive API class - bool reinitialise() { - releaseCurlEngine(); - return initialise(this.keepAlive); - } - // If the API has been configured correctly, print the items that been configured void debugOutputConfiguredAPIItems() { // Debug output of configured URL's @@ -1120,7 +1116,6 @@ class OneDriveApi { // Wrapper function for all requests to OneDrive API // - This should throw a OneDriveException so that this exception can be handled appropriately elsewhere in the application - private JSONValue oneDriveErrorHandlerWrapper(CurlResponse delegate(CurlResponse response) executer, bool validateJSONResponse, string callingFunction, int lineno) { // Create a new 'curl' response response = new CurlResponse(); diff --git a/src/sync.d b/src/sync.d index 01316c5e..65a6fa2b 100644 --- a/src/sync.d +++ b/src/sync.d @@ -185,9 +185,9 @@ class SyncEngine { this(ApplicationConfig appConfig, ItemDatabase itemDB, ClientSideFiltering selectiveSync) { // Create the specific task pool to process items in parallel - this.processPool = new TaskPool(to!int(appConfig.getValueLong("threads"))); + processPool = new TaskPool(to!int(appConfig.getValueLong("threads"))); addLogEntry("PROCESS POOL WORKER THREADS: " ~ to!string(processPool.size), ["debug"]); - + // Configure the class varaible to consume the application configuration this.appConfig = appConfig; // Configure the class varaible to consume the database configuration @@ -302,23 +302,15 @@ class SyncEngine { } } - ~this() { - this.processPool.finish(true); - object.destroy(this.processPool); // Destroy, then set to null - this.processPool = null; - object.destroy(this.appConfig); // Destroy, then set to null - this.appConfig = null; - object.destroy(this.itemDB); // Destroy, then set to null - this.itemDB = null; - object.destroy(this.selectiveSync); // Destroy, then set to null - this.selectiveSync = null; + // The destructor should only clean up resources owned directly by this instance + ~this() { + processPool.finish(true); } // Initialise the Sync Engine class bool initialise() { - // Control whether the worker threads are daemon threads. A daemon thread is automatically terminated when all non-daemon threads have terminated. - processPool.isDaemon(true); + processPool.isDaemon(true); // Create a new instance of the OneDrive API OneDriveApi oneDriveApiInstance; @@ -387,15 +379,15 @@ class SyncEngine { // Shutdown this API instance, as we will create API instances as required, when required oneDriveApiInstance.releaseCurlEngine(); - - // Free object and memory - //object.destroy(oneDriveApiInstance); - //oneDriveApiInstance = null; - - return true; } + // Shutdown the sync engine, wait for anything in processPool to complete + void shutdown() { + addLogEntry("SYNC-ENGINE: Waiting for all internal threads to complete", ["debug"]); + processPool.finish(true); + } + // Get Default Drive Details for this Account void getDefaultDriveDetails() {