From 8bca3644368429e2b4681b0e1f938f1f7af5df82 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Mon, 3 Feb 2020 14:35:14 -0800 Subject: [PATCH 01/11] Collection of fixes for various shutdown scenario race conditions: - possible race condition in WinInet on request cancellation - possible race condition in TPM on task cancellation --- lib/http/HttpClientManager.cpp | 1 + lib/http/HttpClientManager.hpp | 2 +- lib/http/HttpClient_WinInet.cpp | 74 ++++++++--- lib/include/public/ITaskDispatcher.hpp | 9 +- lib/include/public/LogManagerBase.hpp | 1 + lib/include/public/Version.hpp | 8 +- lib/modules | 2 +- lib/pal/TaskDispatcher.hpp | 4 +- lib/pal/TaskDispatcher_CAPI.cpp | 3 +- lib/pal/TaskDispatcher_CAPI.hpp | 2 +- lib/pal/WorkerThread.cpp | 23 +++- lib/pal/desktop/desktop.vcxitems.filters | 2 +- lib/pal/universal/universal.vcxitems.filters | 2 +- lib/tpm/TransmissionPolicyManager.cpp | 124 ++++++++++++------- lib/tpm/TransmissionPolicyManager.hpp | 10 +- tests/functests/APITest.cpp | 80 ++++++------ tests/unittests/UnitTests.vcxproj | 12 +- 17 files changed, 241 insertions(+), 118 deletions(-) diff --git a/lib/http/HttpClientManager.cpp b/lib/http/HttpClientManager.cpp index f25284b59..71017d6a1 100644 --- a/lib/http/HttpClientManager.cpp +++ b/lib/http/HttpClientManager.cpp @@ -106,6 +106,7 @@ namespace ARIASDK_NS_BEGIN { PAL::scheduleTask(&m_taskDispatcher, 0, this, &HttpClientManager::onHttpResponse, callback); } + /* This method may get executed synchronously on Windows from handleSendRequest in case of connection failure */ void HttpClientManager::onHttpResponse(HttpCallback* callback) { EventsUploadContextPtr &ctx = callback->m_ctx; diff --git a/lib/http/HttpClientManager.hpp b/lib/http/HttpClientManager.hpp index be8149987..f59f58a57 100644 --- a/lib/http/HttpClientManager.hpp +++ b/lib/http/HttpClientManager.hpp @@ -51,7 +51,7 @@ class HttpClientManager ILogManager& m_logManager; IHttpClient& m_httpClient; ITaskDispatcher& m_taskDispatcher; - std::mutex m_httpCallbacksMtx; + std::recursive_mutex m_httpCallbacksMtx; std::list m_httpCallbacks; }; diff --git a/lib/http/HttpClient_WinInet.cpp b/lib/http/HttpClient_WinInet.cpp index 86cb1b5d2..2a5831cfc 100644 --- a/lib/http/HttpClient_WinInet.cpp +++ b/lib/http/HttpClient_WinInet.cpp @@ -1,3 +1,4 @@ +// clang-format off // Copyright (c) Microsoft. All rights reserved. #include "mat/config.h" @@ -11,6 +12,7 @@ #include #include +#include #include #include #include @@ -22,17 +24,21 @@ class WinInetRequestWrapper protected: HttpClient_WinInet& m_parent; std::string m_id; - IHttpResponseCallback* m_appCallback {}; - HINTERNET m_hWinInetSession {}; - HINTERNET m_hWinInetRequest {}; + IHttpResponseCallback* m_appCallback{ nullptr }; + HINTERNET m_hWinInetSession { nullptr }; + HINTERNET m_hWinInetRequest { nullptr }; SimpleHttpRequest* m_request; BYTE m_buffer[1024] {}; + std::recursive_mutex m_connectingMutex; + bool isAborted { false }; + bool isCallbackCalled { false }; public: WinInetRequestWrapper(HttpClient_WinInet& parent, SimpleHttpRequest* request) : m_parent(parent), - m_request(request), - m_id(request->GetId()) + m_id(request->GetId()), + m_hWinInetRequest(nullptr), + m_request(request) { LOG_TRACE("%p WinInetRequestWrapper()", this); } @@ -55,10 +61,11 @@ class WinInetRequestWrapper */ void cancel() { + LOCKGUARD(m_connectingMutex); if (m_hWinInetRequest != nullptr) { + isAborted = true; ::InternetCloseHandle(m_hWinInetRequest); - // don't wait for request callback } } @@ -119,6 +126,17 @@ class WinInetRequestWrapper m_parent.m_requests[m_id] = this; } + m_connectingMutex.lock(); + if (isAborted) + { + // Request aborted before being created + m_connectingMutex.unlock(); + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + // Note that at this point the object just self-destroyed. + return; + } + m_connectingMutex.unlock(); + URL_COMPONENTSA urlc; memset(&urlc, 0, sizeof(urlc)); urlc.dwStructSize = sizeof(urlc); @@ -131,7 +149,7 @@ class WinInetRequestWrapper if (!::InternetCrackUrlA(m_request->m_url.data(), (DWORD)m_request->m_url.size(), 0, &urlc)) { DWORD dwError = ::GetLastError(); LOG_WARN("InternetCrackUrl() failed: dwError=%d url=%s", dwError, m_request->m_url.data()); - onRequestComplete(dwError); + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); return; } @@ -140,10 +158,11 @@ class WinInetRequestWrapper if (m_hWinInetSession == NULL) { DWORD dwError = ::GetLastError(); LOG_WARN("InternetConnect() failed: %d", dwError); - onRequestComplete(dwError); + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); return; } // TODO: Session handle for the same target should be cached across requests to enable keep-alive. + DispatchEvent(OnConnecting); PCSTR szAcceptTypes[] = {"*/*", NULL}; m_hWinInetRequest = ::HttpOpenRequestA( @@ -155,9 +174,10 @@ class WinInetRequestWrapper if (m_hWinInetRequest == NULL) { DWORD dwError = ::GetLastError(); LOG_WARN("HttpOpenRequest() failed: %d", dwError); - onRequestComplete(dwError); + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); return; } + DispatchEvent(OnCreated); /* Perform optional MS Root certificate check for certain end-point URLs */ if (m_parent.IsMsRootCheckRequired()) @@ -175,7 +195,13 @@ class WinInetRequestWrapper for (auto const& header : m_request->m_headers) { os << header.first << ": " << header.second << "\r\n"; } - ::HttpAddRequestHeadersA(m_hWinInetRequest, os.str().data(), static_cast(os.tellp()), HTTP_ADDREQ_FLAG_ADD | HTTP_ADDREQ_FLAG_REPLACE); + if (!::HttpAddRequestHeadersA(m_hWinInetRequest, os.str().data(), static_cast(os.tellp()), HTTP_ADDREQ_FLAG_ADD | HTTP_ADDREQ_FLAG_REPLACE)) + { + DWORD dwError = ::GetLastError(); + LOG_WARN("HttpAddRequestHeadersA() failed: %d", dwError); + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + return; + } void *data = static_cast(m_request->m_body.data()); DWORD size = static_cast(m_request->m_body.size()); @@ -185,7 +211,8 @@ class WinInetRequestWrapper if (bResult == TRUE && dwError != ERROR_IO_PENDING) { dwError = ::GetLastError(); LOG_WARN("HttpSendRequest() failed: %d", dwError); - onRequestComplete(dwError); + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + return; } } @@ -207,6 +234,9 @@ class WinInetRequestWrapper return; } + case INTERNET_STATUS_HANDLE_CLOSING: + break; + case INTERNET_STATUS_REQUEST_COMPLETE: { assert(dwStatusInformationLength >= sizeof(INTERNET_ASYNC_RESULT)); INTERNET_ASYNC_RESULT& result = *static_cast(lpvStatusInformation); @@ -222,6 +252,14 @@ class WinInetRequestWrapper } } + void DispatchEvent(HttpStateEvent type) + { + if (m_appCallback != nullptr) + { + m_appCallback->OnHttpStateEvent(type, static_cast(m_hWinInetRequest), 0); + } + } + void onRequestComplete(DWORD dwError) { std::unique_ptr response(new SimpleHttpResponse(m_id)); @@ -348,9 +386,16 @@ class WinInetRequestWrapper } } - // 'response' gets released in EventsUploadContext.clear() - m_appCallback->OnHttpResponse(response.release()); - m_parent.erase(m_id); + assert(isCallbackCalled==false); + if (!isCallbackCalled) + { + // 'response' gets released in EventsUploadContext.clear() + ::InternetSetStatusCallback(m_hWinInetRequest, nullptr); + isCallbackCalled = true; + m_appCallback->OnHttpResponse(response.release()); + // parent is destroying the request object by id + m_parent.erase(m_id); + } } }; @@ -454,3 +499,4 @@ bool HttpClient_WinInet::IsMsRootCheckRequired() } ARIASDK_NS_END #endif // HAVE_MAT_DEFAULT_HTTP_CLIENT +// clang-format on diff --git a/lib/include/public/ITaskDispatcher.hpp b/lib/include/public/ITaskDispatcher.hpp index 181cefb1b..ba0360b90 100644 --- a/lib/include/public/ITaskDispatcher.hpp +++ b/lib/include/public/ITaskDispatcher.hpp @@ -40,7 +40,11 @@ namespace ARIASDK_NS_BEGIN /// /// A Done item is an item that has been marked by the worker thread as already completed. /// - Done + Done, + /// + /// A Cancelled item is an item that has been marked by the worker thread as Cancelled. + /// + Cancelled } Type; /// @@ -92,8 +96,9 @@ namespace ARIASDK_NS_BEGIN /// Cancel a previously queued tasks /// /// Task to be cancelled + /// Amount of time to wait for if the task is currently executing /// True if successfully cancelled, else false - virtual bool Cancel(Task* task) = 0; + virtual bool Cancel(Task* task, uint64_t waitTime = 0) = 0; }; /// @endcond diff --git a/lib/include/public/LogManagerBase.hpp b/lib/include/public/LogManagerBase.hpp index aabb44f10..8dffd0843 100644 --- a/lib/include/public/LogManagerBase.hpp +++ b/lib/include/public/LogManagerBase.hpp @@ -304,6 +304,7 @@ namespace ARIASDK_NS_BEGIN /// static status_t UploadNow() { + LM_LOCKGUARD(stateLock()); if (isHost()) LM_SAFE_CALL(GetLogController()->UploadNow); return STATUS_EPERM; // Permission denied diff --git a/lib/include/public/Version.hpp b/lib/include/public/Version.hpp index 8349e7752..8ab513207 100644 --- a/lib/include/public/Version.hpp +++ b/lib/include/public/Version.hpp @@ -3,8 +3,8 @@ #define MAT_VERSION_HPP // WARNING: DO NOT MODIFY THIS FILE! // This file has been automatically generated, manual changes will be lost. -#define BUILD_VERSION_STR "3.3.8.1" -#define BUILD_VERSION 3,3,8,1 +#define BUILD_VERSION_STR "3.3.29.1" +#define BUILD_VERSION 3,3,29,1 #ifndef RESOURCE_COMPILER_INVOKED #include @@ -30,8 +30,8 @@ namespace ARIASDK_NS_BEGIN { uint64_t const Version = ((uint64_t)3 << 48) | ((uint64_t)3 << 32) | - ((uint64_t)8 << 16) | - ((uint64_t)1 ); + ((uint64_t)29 << 16) | + ((uint64_t)1); } ARIASDK_NS_END diff --git a/lib/modules b/lib/modules index 328df0b08..6ba663a91 160000 --- a/lib/modules +++ b/lib/modules @@ -1 +1 @@ -Subproject commit 328df0b08dd17888e039373e2abb5d2128df136c +Subproject commit 6ba663a91236d8ca8c7adafb5a4f71a7f47a7e82 diff --git a/lib/pal/TaskDispatcher.hpp b/lib/pal/TaskDispatcher.hpp index 9ff24615d..e564f9dd6 100644 --- a/lib/pal/TaskDispatcher.hpp +++ b/lib/pal/TaskDispatcher.hpp @@ -68,11 +68,11 @@ namespace PAL_NS_BEGIN { m_task(h.m_task), m_taskDispatcher(h.m_taskDispatcher) { }; - bool Cancel() + bool Cancel(uint64_t waitTime = 0) { if (m_task) { - bool result = (m_taskDispatcher != nullptr) && (m_taskDispatcher->Cancel(m_task)); + bool result = (m_taskDispatcher != nullptr) && (m_taskDispatcher->Cancel(m_task, waitTime)); m_task = nullptr; m_taskDispatcher = nullptr; return result; diff --git a/lib/pal/TaskDispatcher_CAPI.cpp b/lib/pal/TaskDispatcher_CAPI.cpp index 5b99a4666..8aa8bdea7 100644 --- a/lib/pal/TaskDispatcher_CAPI.cpp +++ b/lib/pal/TaskDispatcher_CAPI.cpp @@ -125,7 +125,8 @@ namespace PAL_NS_BEGIN { m_queueFn(&capiTask, &OnAsyncTaskCallback); } - bool TaskDispatcher_CAPI::Cancel(Task* task) + // TODO: currently shutdown wait on task cancellation is not implemented for C API Task Dispatcher + bool TaskDispatcher_CAPI::Cancel(Task* task, uint64_t) { std::string taskId; diff --git a/lib/pal/TaskDispatcher_CAPI.hpp b/lib/pal/TaskDispatcher_CAPI.hpp index d7af9b530..66d22ddf7 100644 --- a/lib/pal/TaskDispatcher_CAPI.hpp +++ b/lib/pal/TaskDispatcher_CAPI.hpp @@ -12,7 +12,7 @@ namespace PAL_NS_BEGIN { TaskDispatcher_CAPI(task_dispatcher_queue_fn_t queueFn, task_dispatcher_cancel_fn_t cancelFn, task_dispatcher_join_fn_t joinFn); void Join() override; void Queue(MAT::Task* task) override; - bool Cancel(MAT::Task* task) override; + bool Cancel(MAT::Task* task, uint64_t waitTime = 0) override; private: task_dispatcher_queue_fn_t m_queueFn; diff --git a/lib/pal/WorkerThread.cpp b/lib/pal/WorkerThread.cpp index df9b0283d..1a59177a4 100644 --- a/lib/pal/WorkerThread.cpp +++ b/lib/pal/WorkerThread.cpp @@ -6,6 +6,9 @@ /* Maximum scheduler interval for SDK is 1 hour required for clamping in case of monotonic clock drift */ #define MAX_FUTURE_DELTA_MS (60 * 60 * 1000) +/* Polling interval for task cancellation */ +#define TASK_CANCEL_WAIT_MS 100 + namespace PAL_NS_BEGIN { class WorkerThreadShutdownItem : public Task @@ -88,13 +91,29 @@ namespace PAL_NS_BEGIN { m_event.post(); } - bool Cancel(MAT::Task* item) override + bool Cancel(MAT::Task* item, uint64_t waitTime) override { - if ((m_itemInProgress == item)||(item==nullptr)) + if (item == nullptr) { return false; } + if (m_itemInProgress == item) + { + /* Can't recursively wait on completion of our own thread */ + if (m_hThread.get_id() != std::this_thread::get_id()) + { + LOCKGUARD(m_lock); + while ((waitTime > TASK_CANCEL_WAIT_MS) && (m_itemInProgress == item)) + { + PAL::sleep(TASK_CANCEL_WAIT_MS); + waitTime -= TASK_CANCEL_WAIT_MS; + } + /* Either waited long enough or the task is still executing */ + } + return (m_itemInProgress == item); + } + { LOCKGUARD(m_lock); auto it = std::find(m_timerQueue.begin(), m_timerQueue.end(), item); diff --git a/lib/pal/desktop/desktop.vcxitems.filters b/lib/pal/desktop/desktop.vcxitems.filters index 85e227743..a1d6dd857 100644 --- a/lib/pal/desktop/desktop.vcxitems.filters +++ b/lib/pal/desktop/desktop.vcxitems.filters @@ -10,7 +10,7 @@ - + diff --git a/lib/pal/universal/universal.vcxitems.filters b/lib/pal/universal/universal.vcxitems.filters index 10c6a8b55..166fe746a 100644 --- a/lib/pal/universal/universal.vcxitems.filters +++ b/lib/pal/universal/universal.vcxitems.filters @@ -11,7 +11,7 @@ - + diff --git a/lib/tpm/TransmissionPolicyManager.cpp b/lib/tpm/TransmissionPolicyManager.cpp index f8a7a882d..66ff864cc 100644 --- a/lib/tpm/TransmissionPolicyManager.cpp +++ b/lib/tpm/TransmissionPolicyManager.cpp @@ -6,16 +6,16 @@ #include -#define ABS64(a,b) ((a>b)?(a-b):(b-a)) +#define ABS64(a, b) ((a > b) ? (a - b) : (b - a)) -namespace ARIASDK_NS_BEGIN { - - int const DEFAULT_DELAY_SEND_HTTP = 2 * 1000; // 2 sec +namespace ARIASDK_NS_BEGIN +{ + int const DEFAULT_DELAY_SEND_HTTP = 2 * 1000; // 2 sec MATSDK_LOG_INST_COMPONENT_CLASS(TransmissionPolicyManager, "EventsSDK.TPM", "Events telemetry client - TransmissionPolicyManager class"); - TransmissionPolicyManager::TransmissionPolicyManager(ITelemetrySystem& system, ITaskDispatcher& taskDispatcher, IBandwidthController* bandwidthController) - : m_lock(), + TransmissionPolicyManager::TransmissionPolicyManager(ITelemetrySystem& system, ITaskDispatcher& taskDispatcher, IBandwidthController* bandwidthController) : + m_lock(), m_system(system), m_taskDispatcher(taskDispatcher), m_config(m_system.getConfig()), @@ -24,7 +24,8 @@ namespace ARIASDK_NS_BEGIN { m_isUploadScheduled(false), m_scheduledUploadTime(std::numeric_limits::max()), m_timerdelay(DEFAULT_DELAY_SEND_HTTP), - m_runningLatency(EventLatency_RealTime) + m_runningLatency(EventLatency_RealTime), + m_scheduledUploadAborted(false) { m_backoffConfig = "E,3000,300000,2,1"; m_backoff = IBackoff::createFromConfig(m_backoffConfig); @@ -39,24 +40,53 @@ namespace ARIASDK_NS_BEGIN { void TransmissionPolicyManager::checkBackoffConfigUpdate() { + LOCKGUARD(m_backoff_lock); std::string config = m_config.GetUploadRetryBackoffConfig(); - if (config != m_backoffConfig) { + if (config != m_backoffConfig) + { std::unique_ptr backoff = IBackoff::createFromConfig(config); - if (!backoff) { + if (!backoff) + { LOG_WARN("The new backoff configuration is invalid, continuing to use current settings"); } - else { + else + { m_backoff = std::move(backoff); m_backoffConfig = config; } } } + void TransmissionPolicyManager::resetBackoff() + { + LOCKGUARD(m_backoff_lock); + if (m_backoff) + m_backoff->reset(); + } + + int TransmissionPolicyManager::increaseBackoff() + { + int delayMs = 0; + LOCKGUARD(m_backoff_lock); + checkBackoffConfigUpdate(); + if (m_backoff) + { + delayMs = m_backoff->getValue(); + m_backoff->increase(); + } + return delayMs; + } + // TODO: consider changing int delayInMs to std::chrono::duration<> in millis. // The duration delayInMs passed to that function must be always >= 0 ms void TransmissionPolicyManager::scheduleUpload(int delayInMs, EventLatency latency, bool force) { - if (uploadCount() >= static_cast(m_config[CFG_INT_MAX_PENDING_REQ]) ) + LOCKGUARD(m_scheduledUploadMutex); + if (m_scheduledUploadAborted) + { + return; + } + if (uploadCount() >= static_cast(m_config[CFG_INT_MAX_PENDING_REQ])) { LOG_TRACE("Maximum number of HTTP requests reached"); return; @@ -68,7 +98,7 @@ namespace ARIASDK_NS_BEGIN { return; } - if ((!force)&&(m_isUploadScheduled)) + if ((!force) && (m_isUploadScheduled)) { if (m_runningLatency > latency) { @@ -109,29 +139,36 @@ namespace ARIASDK_NS_BEGIN { void TransmissionPolicyManager::uploadAsync(EventLatency latency) { - m_isUploadScheduled = false; // Allow to schedule another uploadAsync + m_isUploadScheduled = false; // Allow to schedule another uploadAsync m_runningLatency = latency; m_scheduledUploadTime = std::numeric_limits::max(); - if (m_isPaused) { - LOG_TRACE("Paused, not uploading anything until resumed"); - cancelUploadTask(); // If there is a pending upload task, kill it - return; + { + LOCKGUARD(m_scheduledUploadMutex); + if ((m_isPaused) || (m_scheduledUploadAborted)) + { + LOG_TRACE("Paused, not uploading anything until resumed"); + cancelUploadTask(); // If there is a pending upload task, kill it + return; + } } -#ifdef ENABLE_BW_CONTROLLER /* Bandwidth controller is not currently supported */ - if (m_bandwidthController) { +#ifdef ENABLE_BW_CONTROLLER /* Bandwidth controller is not currently supported */ + if (m_bandwidthController) + { unsigned proposedBandwidthBps = m_bandwidthController->GetProposedBandwidthBps(); unsigned minimumBandwidthBps = m_config.GetMinimumUploadBandwidthBps(); - if (proposedBandwidthBps >= minimumBandwidthBps) { + if (proposedBandwidthBps >= minimumBandwidthBps) + { LOG_TRACE("Bandwidth controller proposed sufficient bandwidth %u bytes/sec (minimum accepted is %u)", - proposedBandwidthBps, minimumBandwidthBps); + proposedBandwidthBps, minimumBandwidthBps); } - else { + else + { unsigned delayMs = 1000; LOG_INFO("Bandwidth controller proposed bandwidth %u bytes/sec but minimum accepted is %u, will retry %u ms later", - proposedBandwidthBps, minimumBandwidthBps, delayMs); - scheduleUpload(delayMs, latency); // reschedule uploadAsync to run again 1000 ms later + proposedBandwidthBps, minimumBandwidthBps, delayMs); + scheduleUpload(delayMs, latency); // reschedule uploadAsync to run again 1000 ms later return; } } @@ -156,7 +193,7 @@ namespace ARIASDK_NS_BEGIN { { LOG_TRACE("Scheduling upload in %d ms", nextUploadInMs); EventLatency proposed = calculateNewPriority(); - scheduleUpload(nextUploadInMs, proposed); // reschedule uploadAsync again + scheduleUpload(nextUploadInMs, proposed); // reschedule uploadAsync again } } @@ -190,6 +227,15 @@ namespace ARIASDK_NS_BEGIN { */ bool TransmissionPolicyManager::handleStop() { + { + LOCKGUARD(m_scheduledUploadMutex); + // Prevent execution of all upload tasks + m_scheduledUploadAborted = true; + } + + // Make sure we wait for completion on the upload scheduling task that may be running + cancelUploadTask(); + // Make sure we wait for all active upload callbacks to finish while (uploadCount() > 0) { std::this_thread::yield(); @@ -202,19 +248,21 @@ namespace ARIASDK_NS_BEGIN { void TransmissionPolicyManager::handleFinishAllUploads() { pauseAllUploads(); - allUploadsFinished(); // calls stats.onStop >> this->flushTaskDispatcher; + allUploadsFinished(); // calls stats.onStop >> this->flushTaskDispatcher; } void TransmissionPolicyManager::handleEventArrived(IncomingEventContextPtr const& event) { - if (m_isPaused) { + if (m_isPaused) + { return; } bool forceTimerRestart = false; /* This logic needs to be revised: one event in a dedicated HTTP post is wasteful! */ // Initiate upload right away - if (event->record.latency > EventLatency_RealTime) { + if (event->record.latency > EventLatency_RealTime) + { EventsUploadContextPtr ctx = new EventsUploadContext(); ctx->requestedMinLatency = event->record.latency; addUpload(ctx); @@ -271,7 +319,7 @@ namespace ARIASDK_NS_BEGIN { void TransmissionPolicyManager::handleNothingToUpload(EventsUploadContextPtr const& ctx) { LOG_TRACE("No stored events to send at the moment"); - m_backoff->reset(); + resetBackoff(); if (ctx->requestedMinLatency == EventLatency_Normal) { finishUpload(ctx, -1); @@ -289,26 +337,18 @@ namespace ARIASDK_NS_BEGIN { void TransmissionPolicyManager::handleEventsUploadSuccessful(EventsUploadContextPtr const& ctx) { - m_backoff->reset(); + resetBackoff(); finishUpload(ctx, 0); } void TransmissionPolicyManager::handleEventsUploadRejected(EventsUploadContextPtr const& ctx) { - checkBackoffConfigUpdate(); - int delayMs = m_backoff->getValue(); - m_backoff->increase(); - - finishUpload(ctx, delayMs); + finishUpload(ctx, increaseBackoff()); } void TransmissionPolicyManager::handleEventsUploadFailed(EventsUploadContextPtr const& ctx) { - checkBackoffConfigUpdate(); - int delayMs = m_backoff->getValue(); - m_backoff->increase(); - - finishUpload(ctx, delayMs); + finishUpload(ctx, increaseBackoff()); } void TransmissionPolicyManager::handleEventsUploadAborted(EventsUploadContextPtr const& ctx) @@ -316,5 +356,5 @@ namespace ARIASDK_NS_BEGIN { finishUpload(ctx, -1); } - -} ARIASDK_NS_END +} +ARIASDK_NS_END diff --git a/lib/tpm/TransmissionPolicyManager.hpp b/lib/tpm/TransmissionPolicyManager.hpp index 14881956e..e6616edab 100644 --- a/lib/tpm/TransmissionPolicyManager.hpp +++ b/lib/tpm/TransmissionPolicyManager.hpp @@ -16,6 +16,7 @@ #include #include +#include namespace ARIASDK_NS_BEGIN { @@ -30,6 +31,8 @@ namespace ARIASDK_NS_BEGIN { protected: MATSDK_LOG_DECL_COMPONENT_CLASS(); void checkBackoffConfigUpdate(); + void resetBackoff(); + int increaseBackoff(); void uploadAsync(EventLatency priority); void finishUpload(EventsUploadContextPtr ctx, int nextUploadInMs); @@ -57,6 +60,7 @@ namespace ARIASDK_NS_BEGIN { IRuntimeConfig& m_config; IBandwidthController* m_bandwidthController; + std::recursive_mutex m_backoff_lock; std::string m_backoffConfig; // TODO: [MG] - move to config std::unique_ptr m_backoff; DeviceStateHandler m_deviceStateHandler; @@ -64,7 +68,10 @@ namespace ARIASDK_NS_BEGIN { std::atomic m_isPaused; std::atomic m_isUploadScheduled; uint64_t m_scheduledUploadTime; + + std::mutex m_scheduledUploadMutex; PAL::DeferredCallbackHandle m_scheduledUpload; + bool m_scheduledUploadAborted; std::mutex m_activeUploads_lock; std::set m_activeUploads; @@ -112,7 +119,8 @@ namespace ARIASDK_NS_BEGIN { /// bool cancelUploadTask() { - bool result = m_scheduledUpload.Cancel(); + uint64_t cancelWaitTimeMs = (m_scheduledUploadAborted) ? UINT64_MAX : 0; + bool result = m_scheduledUpload.Cancel(cancelWaitTimeMs); m_isUploadScheduled.exchange(false); return result; } diff --git a/tests/functests/APITest.cpp b/tests/functests/APITest.cpp index 63f8554c1..c1f6357e9 100644 --- a/tests/functests/APITest.cpp +++ b/tests/functests/APITest.cpp @@ -1064,6 +1064,47 @@ TEST(APITest, LogManager_BadStoragePath_Test) } +#ifdef HAVE_MAT_WININET_HTTP_CLIENT +/* This test requires WinInet HTTP client */ +TEST(APITest, LogConfiguration_MsRoot_Check) +{ + TestDebugEventListener debugListener; + std::list> testParams = + { + {"https://v10.events.data.microsoft.com/OneCollector/1.0/", false, 1}, // MS-Rooted, no MS-Root check: post succeeds + {"https://v10.events.data.microsoft.com/OneCollector/1.0/", true, 1}, // MS-Rooted, MS-Root check: post succeeds + {"https://self.events.data.microsoft.com/OneCollector/1.0/", false, 1}, // Non-MS rooted, no MS-Root check: post succeeds + {"https://self.events.data.microsoft.com/OneCollector/1.0/", true, 0} // Non-MS rooted, MS-Root check: post fails + }; + + // 4 test runs + for (const auto& params : testParams) + { + CleanStorage(); + + auto& config = LogManager::GetLogConfiguration(); + config["stats"]["interval"] = 0; // avoid sending stats for this test, just customer events + config[CFG_STR_COLLECTOR_URL] = std::get<0>(params); + config["http"]["msRootCheck"] = std::get<1>(params); // MS root check depends on what URL we are sending to + config[CFG_INT_MAX_TEARDOWN_TIME] = 1; // up to 1s wait to perform HTTP post on teardown + config[CFG_STR_CACHE_FILE_PATH] = GetStoragePath(); + auto expectedHttpCount = std::get<2>(params); + + auto logger = LogManager::Initialize(TEST_TOKEN, config); + + debugListener.reset(); + addAllListeners(debugListener); + logger->LogEvent("fooBar"); + LogManager::FlushAndTeardown(); + removeAllListeners(debugListener); + + // Connection is a best-effort, occasionally we can't connect, + // but we MUST NOT connect to end-point that doesn't have the + // right cert. + EXPECT_LE(debugListener.numHttpOK, expectedHttpCount); + } +} +#endif TEST(APITest, LogManager_BadNetwork_Test) { auto& config = LogManager::GetLogConfiguration(); @@ -1102,45 +1143,6 @@ TEST(APITest, LogManager_BadNetwork_Test) } } -#ifdef HAVE_MAT_WININET_HTTP_CLIENT -/* This test requires WinInet HTTP client */ -TEST(APITest, LogConfiguration_MsRoot_Check) -{ - TestDebugEventListener debugListener; - std::list> testParams = - { - { "https://v10.events.data.microsoft.com/OneCollector/1.0/", false, 1}, // MS-Rooted, no MS-Root check: post succeeds - { "https://v10.events.data.microsoft.com/OneCollector/1.0/", true, 1}, // MS-Rooted, MS-Root check: post succeeds - { "https://self.events.data.microsoft.com/OneCollector/1.0/", false, 1}, // Non-MS rooted, no MS-Root check: post succeeds - { "https://self.events.data.microsoft.com/OneCollector/1.0/", true, 0} // Non-MS rooted, MS-Root check: post fails - }; - - // 4 test runs - for (const auto ¶ms : testParams) - { - CleanStorage(); - - auto& config = LogManager::GetLogConfiguration(); - config["stats"]["interval"] = 0; // avoid sending stats for this test, just customer events - config[CFG_STR_COLLECTOR_URL] = std::get<0>(params); - config["http"]["msRootCheck"] = std::get<1>(params); // MS root check depends on what URL we are sending to - config[CFG_INT_MAX_TEARDOWN_TIME] = 1; // up to 1s wait to perform HTTP post on teardown - config[CFG_STR_CACHE_FILE_PATH] = GetStoragePath(); - auto expectedHttpCount = std::get<2>(params); - - auto logger = LogManager::Initialize(TEST_TOKEN, config); - - debugListener.reset(); - addAllListeners(debugListener); - logger->LogEvent("fooBar"); - LogManager::FlushAndTeardown(); - removeAllListeners(debugListener); - - EXPECT_EQ(debugListener.numHttpOK, expectedHttpCount); - } -} -#endif - TEST(APITest, LogManager_GetLoggerSameLoggerMultithreaded) { auto& config = LogManager::GetLogConfiguration(); diff --git a/tests/unittests/UnitTests.vcxproj b/tests/unittests/UnitTests.vcxproj index de94b11a2..73c5ec321 100644 --- a/tests/unittests/UnitTests.vcxproj +++ b/tests/unittests/UnitTests.vcxproj @@ -164,7 +164,7 @@ /machine:X86 %(AdditionalOptions) - pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) @@ -212,7 +212,7 @@ /machine:X86 %(AdditionalOptions) - pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) @@ -258,7 +258,7 @@ /machine:X64 %(AdditionalOptions) - pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) @@ -306,7 +306,7 @@ /machine:ARM64 %(AdditionalOptions) - pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) @@ -354,7 +354,7 @@ /machine:X64 %(AdditionalOptions) - pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) @@ -400,7 +400,7 @@ /machine:ARM64 %(AdditionalOptions) - pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) From 80d7a8e32d255919512c9a98258870cd49f772b3 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Wed, 5 Feb 2020 17:06:36 -0800 Subject: [PATCH 02/11] Fix FlushAndTeardown causing double decrement on PAL ref-count. Add stress test that inspects PAL ref-count. Rework the safety checks in LogManagerBase.hpp. Add multiple log managers stress-test with concurrent UploadNow. --- lib/api/LogManagerImpl.cpp | 18 +- lib/http/HttpClient_WinInet.cpp | 177 ++++++---- lib/include/public/LogManagerBase.hpp | 228 +++++++------ lib/pal/PAL.hpp | 41 ++- tests/functests/BasicFuncTests.cpp | 466 +++++++++++++++++--------- 5 files changed, 576 insertions(+), 354 deletions(-) diff --git a/lib/api/LogManagerImpl.cpp b/lib/api/LogManagerImpl.cpp index 109c165d4..b1f156a3d 100644 --- a/lib/api/LogManagerImpl.cpp +++ b/lib/api/LogManagerImpl.cpp @@ -292,10 +292,9 @@ namespace ARIASDK_NS_BEGIN void LogManagerImpl::FlushAndTeardown() { LOG_INFO("Shutting down..."); - + LOCKGUARD(m_lock); + if (m_alive) { - LOCKGUARD(m_lock); - LOG_INFO("Tearing down modules"); TeardownModules(); @@ -315,15 +314,14 @@ namespace ARIASDK_NS_BEGIN m_httpClient = nullptr; m_taskDispatcher = nullptr; m_dataViewer = nullptr; - } - m_filters.UnregisterAllFilters(); - - auto shutTime = GetUptimeMs(); - PAL::shutdown(); - shutTime = GetUptimeMs() - shutTime; - LOG_INFO("Shutdown complete in %lld ms", shutTime); + m_filters.UnregisterAllFilters(); + auto shutTime = GetUptimeMs(); + PAL::shutdown(); + shutTime = GetUptimeMs() - shutTime; + LOG_INFO("Shutdown complete in %lld ms", shutTime); + } m_alive = false; } diff --git a/lib/http/HttpClient_WinInet.cpp b/lib/http/HttpClient_WinInet.cpp index 2a5831cfc..29da9409e 100644 --- a/lib/http/HttpClient_WinInet.cpp +++ b/lib/http/HttpClient_WinInet.cpp @@ -62,9 +62,11 @@ class WinInetRequestWrapper void cancel() { LOCKGUARD(m_connectingMutex); + // Cover the case where m_hWinInetRequest handle hasn't been created yet: + isAborted = true; + // Cover the case where m_hWinInetRequest handle is created and has a callback registered: if (m_hWinInetRequest != nullptr) { - isAborted = true; ::InternetCloseHandle(m_hWinInetRequest); } } @@ -119,6 +121,7 @@ class WinInetRequestWrapper void send(IHttpResponseCallback* callback) { + // Register app callback and request in HttpClient map m_appCallback = callback; { @@ -126,83 +129,115 @@ class WinInetRequestWrapper m_parent.m_requests[m_id] = this; } - m_connectingMutex.lock(); - if (isAborted) - { - // Request aborted before being created - m_connectingMutex.unlock(); - onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); - // Note that at this point the object just self-destroyed. - return; - } - m_connectingMutex.unlock(); - - URL_COMPONENTSA urlc; - memset(&urlc, 0, sizeof(urlc)); - urlc.dwStructSize = sizeof(urlc); - char hostname[256]; - urlc.lpszHostName = hostname; - urlc.dwHostNameLength = sizeof(hostname); - char path[1024]; - urlc.lpszUrlPath = path; - urlc.dwUrlPathLength = sizeof(path); - if (!::InternetCrackUrlA(m_request->m_url.data(), (DWORD)m_request->m_url.size(), 0, &urlc)) { - DWORD dwError = ::GetLastError(); - LOG_WARN("InternetCrackUrl() failed: dwError=%d url=%s", dwError, m_request->m_url.data()); - onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); - return; - } + { // Section of code that uses m_connectingMutex lock without a LOCKGUARD macro: + // - don't forget to unlock the mutex before calling "onRequestComplete" which destroys this object + // - don't access any object members after "onRequestComplete" (object is dead at this point) + m_connectingMutex.lock(); - m_hWinInetSession = ::InternetConnectA(m_parent.m_hInternet, hostname, urlc.nPort, - NULL, NULL, INTERNET_SERVICE_HTTP, 0, reinterpret_cast(this)); - if (m_hWinInetSession == NULL) { - DWORD dwError = ::GetLastError(); - LOG_WARN("InternetConnect() failed: %d", dwError); - onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); - return; - } - // TODO: Session handle for the same target should be cached across requests to enable keep-alive. - DispatchEvent(OnConnecting); - - PCSTR szAcceptTypes[] = {"*/*", NULL}; - m_hWinInetRequest = ::HttpOpenRequestA( - m_hWinInetSession, m_request->m_method.c_str(), path, NULL, NULL, szAcceptTypes, - INTERNET_FLAG_KEEP_CONNECTION | INTERNET_FLAG_NO_AUTH | INTERNET_FLAG_NO_CACHE_WRITE | - INTERNET_FLAG_NO_COOKIES | INTERNET_FLAG_NO_UI | INTERNET_FLAG_PRAGMA_NOCACHE | - INTERNET_FLAG_RELOAD | (urlc.nScheme == INTERNET_SCHEME_HTTPS ? INTERNET_FLAG_SECURE : 0), - reinterpret_cast(this)); - if (m_hWinInetRequest == NULL) { - DWORD dwError = ::GetLastError(); - LOG_WARN("HttpOpenRequest() failed: %d", dwError); - onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); - return; - } - DispatchEvent(OnCreated); + // If outside code asked us to abort that request before we could proceed with creating a WinInet + // handle on it, then clean it right away before proceeding with any async WinInet API calls. + if (isAborted) + { + // Request force-aborted before creating a WinInet handle. + DispatchEvent(OnConnectFailed); + m_connectingMutex.unlock(); + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + return; + } - /* Perform optional MS Root certificate check for certain end-point URLs */ - if (m_parent.IsMsRootCheckRequired()) - { - if (!isMsRootCert()) + DispatchEvent(OnConnecting); + + URL_COMPONENTSA urlc; + memset(&urlc, 0, sizeof(urlc)); + urlc.dwStructSize = sizeof(urlc); + char hostname[256]; + urlc.lpszHostName = hostname; + urlc.dwHostNameLength = sizeof(hostname); + char path[1024]; + urlc.lpszUrlPath = path; + urlc.dwUrlPathLength = sizeof(path); + if (!::InternetCrackUrlA(m_request->m_url.data(), (DWORD)m_request->m_url.size(), 0, &urlc)) { - onRequestComplete(ERROR_INTERNET_SEC_INVALID_CERT); + DWORD dwError = ::GetLastError(); + LOG_WARN("InternetCrackUrl() failed: dwError=%d url=%s", dwError, m_request->m_url.data()); + // Invalid URL passed to WinInet API + DispatchEvent(OnConnectFailed); + m_connectingMutex.unlock(); + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); return; } - } - ::InternetSetStatusCallback(m_hWinInetRequest, &WinInetRequestWrapper::winInetCallback); + m_hWinInetSession = ::InternetConnectA(m_parent.m_hInternet, hostname, urlc.nPort, + NULL, NULL, INTERNET_SERVICE_HTTP, 0, reinterpret_cast(this)); + if (m_hWinInetSession == NULL) + { + DWORD dwError = ::GetLastError(); + LOG_WARN("InternetConnect() failed: %d", dwError); + // Cannot connect to host + DispatchEvent(OnConnectFailed); + m_connectingMutex.unlock(); + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + return; + } + // TODO: Session handle for the same target should be cached across requests to enable keep-alive. + + PCSTR szAcceptTypes[] = {"*/*", NULL}; + m_hWinInetRequest = ::HttpOpenRequestA( + m_hWinInetSession, m_request->m_method.c_str(), path, NULL, NULL, szAcceptTypes, + INTERNET_FLAG_KEEP_CONNECTION | INTERNET_FLAG_NO_AUTH | INTERNET_FLAG_NO_CACHE_WRITE | + INTERNET_FLAG_NO_COOKIES | INTERNET_FLAG_NO_UI | INTERNET_FLAG_PRAGMA_NOCACHE | + INTERNET_FLAG_RELOAD | (urlc.nScheme == INTERNET_SCHEME_HTTPS ? INTERNET_FLAG_SECURE : 0), + reinterpret_cast(this)); + if (m_hWinInetRequest == NULL) + { + DWORD dwError = ::GetLastError(); + LOG_WARN("HttpOpenRequest() failed: %d", dwError); + // Request cannot be opened to given URL because of some connectivity issue + DispatchEvent(OnConnectFailed); + m_connectingMutex.unlock(); + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + return; + } + + /* Perform optional MS Root certificate check for certain end-point URLs */ + if (m_parent.IsMsRootCheckRequired()) + { + if (!isMsRootCert()) + { + // Request cannot be completed: end-point certificate is not MS-Rooted. + DispatchEvent(OnConnectFailed); + m_connectingMutex.unlock(); + onRequestComplete(ERROR_INTERNET_SEC_INVALID_CERT); + return; + } + } + + ::InternetSetStatusCallback(m_hWinInetRequest, &WinInetRequestWrapper::winInetCallback); + // At this point this->cancel() is able to async-destroy the request object by calling + // ::InternetCloseHandle(...) : winInetCallback is invoked in context of WinInet thread + // pool worker thread, which subsequently calls onRequestComplete(...) to destroy. + m_connectingMutex.unlock(); + } + // End of section of code that uses m_connectingMutex lock std::ostringstream os; - for (auto const& header : m_request->m_headers) { + for (auto const& header : m_request->m_headers) + { os << header.first << ": " << header.second << "\r\n"; } if (!::HttpAddRequestHeadersA(m_hWinInetRequest, os.str().data(), static_cast(os.tellp()), HTTP_ADDREQ_FLAG_ADD | HTTP_ADDREQ_FLAG_REPLACE)) { DWORD dwError = ::GetLastError(); LOG_WARN("HttpAddRequestHeadersA() failed: %d", dwError); + // Unable to add request headers. There's no point in proceeding with upload because + // our server is expecting those custom request headers to always be there. + DispatchEvent(OnConnectFailed); onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); return; } + // Try to send headers and request body to server + DispatchEvent(OnSending); void *data = static_cast(m_request->m_body.data()); DWORD size = static_cast(m_request->m_body.size()); BOOL bResult = ::HttpSendRequest(m_hWinInetRequest, NULL, 0, data, (DWORD)size); @@ -211,9 +246,12 @@ class WinInetRequestWrapper if (bResult == TRUE && dwError != ERROR_IO_PENDING) { dwError = ::GetLastError(); LOG_WARN("HttpSendRequest() failed: %d", dwError); + // Unable to send requerst + DispatchEvent(OnSendFailed); onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); return; } + // Async request has been queued in WinInet thread pool } static void CALLBACK winInetCallback(HINTERNET hInternet, DWORD_PTR dwContext, DWORD dwInternetStatus, LPVOID lpvStatusInformation, DWORD dwStatusInformationLength) @@ -235,7 +273,10 @@ class WinInetRequestWrapper } case INTERNET_STATUS_HANDLE_CLOSING: - break; + // HANDLE_CLOSING should always come after REQUEST_COMPLETE/. When (and if) + // it (ever) happens, WinInetRequestWrapper* self pointer may point to object + // that has been already destroyed. We do not perform any actions on it. + return; case INTERNET_STATUS_REQUEST_COMPLETE: { assert(dwStatusInformationLength >= sizeof(INTERNET_ASYNC_RESULT)); @@ -295,6 +336,7 @@ class WinInetRequestWrapper } while (m_bufferUsed == sizeof(m_buffer)); } + // SUCCESS with no IO_PENDING means we're done with the response body: try to parse the response headers. if (dwError == ERROR_SUCCESS) { response->m_result = HttpResult_OK; @@ -346,7 +388,10 @@ class WinInetRequestWrapper response->m_headers.add(name, value1); ptr = eol + 2; } - + // This event handler covers the only positive case when we actually got some server response. + // We may still invoke OnHttpResponse(...) below for this positive as well as other negative + // cases where there was a short-read, connection failuire or timeout on reading the response. + DispatchEvent(OnResponse); } else { switch (dwError) { case ERROR_INTERNET_OPERATION_CANCELLED: @@ -389,11 +434,13 @@ class WinInetRequestWrapper assert(isCallbackCalled==false); if (!isCallbackCalled) { - // 'response' gets released in EventsUploadContext.clear() - ::InternetSetStatusCallback(m_hWinInetRequest, nullptr); + // Only one WinInet worker thread may invoke async callback for a given request at any given moment of time. + // That ensures that isCallbackCalled does not require a lock around it. We unregister the callback here + // to ensure that no more callbacks are coming for that m_hWinInetRequest. + ::InternetSetStatusCallback(m_hWinInetRequest, NULL); isCallbackCalled = true; m_appCallback->OnHttpResponse(response.release()); - // parent is destroying the request object by id + // HttpClient parent is destroying this HttpRequest object by id m_parent.erase(m_id); } } diff --git a/lib/include/public/LogManagerBase.hpp b/lib/include/public/LogManagerBase.hpp index 8dffd0843..70965b904 100644 --- a/lib/include/public/LogManagerBase.hpp +++ b/lib/include/public/LogManagerBase.hpp @@ -2,8 +2,8 @@ #ifndef MAT_LOGMANAGER_HPP #define MAT_LOGMANAGER_HPP -#include "ctmacros.hpp" #include "CommonFields.h" +#include "ctmacros.hpp" #if (HAVE_EXCEPTIONS) #include @@ -11,7 +11,7 @@ #ifdef _MSC_VER #pragma warning(push) -#pragma warning(disable:4459 4100 4121 4068) +#pragma warning(disable : 4459 4100 4121 4068) #endif #pragma clang diagnostic push @@ -19,8 +19,10 @@ #ifdef _MANAGED #include -public ref class LogManagerLock { -public: +public +ref class LogManagerLock +{ + public: static Object ^ lock = gcnew Object(); }; #else @@ -29,71 +31,71 @@ public ref class LogManagerLock { #include "LogManagerProvider.hpp" -#define LM_SAFE_CALL(method , ... ) \ - { \ - LM_LOCKGUARD(stateLock()); \ - if (nullptr != instance) \ - { \ - instance-> method ( __VA_ARGS__); \ - return STATUS_SUCCESS; \ - } \ - return STATUS_EFAIL; \ +#define LM_SAFE_CALL(method, ...) \ + { \ + LM_LOCKGUARD(stateLock()); \ + if (nullptr != instance) \ + { \ + instance->method(__VA_ARGS__); \ + return STATUS_SUCCESS; \ + } \ + return STATUS_EFAIL; \ } -#define LM_SAFE_CALL_RETURN(method , ... ) \ - { \ - LM_LOCKGUARD(stateLock()); \ - if (nullptr != instance) \ - { \ - return instance-> method ( __VA_ARGS__);\ - } \ +#define LM_SAFE_CALL_RETURN(method, ...) \ + { \ + LM_LOCKGUARD(stateLock()); \ + if (nullptr != instance) \ + { \ + return instance->method(__VA_ARGS__); \ + } \ } -#define LM_SAFE_CALL_STR(method , ... ) \ - { \ - LM_LOCKGUARD(stateLock()); \ - if (nullptr != instance) \ - { \ - return instance-> method ( __VA_ARGS__);\ - } \ - return ""; \ +#define LM_SAFE_CALL_STR(method, ...) \ + { \ + LM_LOCKGUARD(stateLock()); \ + if (nullptr != instance) \ + { \ + return instance->method(__VA_ARGS__); \ + } \ + return ""; \ } -#define LM_SAFE_CALL_PTR(method , ... ) \ - { \ - LM_LOCKGUARD(stateLock()); \ - if (nullptr != instance) \ - { \ - return instance-> method ( __VA_ARGS__);\ - } \ - return nullptr; \ +#define LM_SAFE_CALL_PTR(method, ...) \ + { \ + LM_LOCKGUARD(stateLock()); \ + if (nullptr != instance) \ + { \ + return instance->method(__VA_ARGS__); \ + } \ + return nullptr; \ } -#define LM_SAFE_CALL_PTRREF(method , ... ) \ - { \ - LM_LOCKGUARD(stateLock()); \ - if (nullptr != instance) \ - { \ - return &(instance-> method ( __VA_ARGS__));\ - } \ - return nullptr; \ +#define LM_SAFE_CALL_PTRREF(method, ...) \ + { \ + LM_LOCKGUARD(stateLock()); \ + if (nullptr != instance) \ + { \ + return &(instance->method(__VA_ARGS__)); \ + } \ + return nullptr; \ } -#define LM_SAFE_CALL_VOID(method , ... ) \ - { \ - LM_LOCKGUARD(stateLock()); \ - if (nullptr != instance) \ - { \ - instance-> method ( __VA_ARGS__); \ - return; \ - } \ +#define LM_SAFE_CALL_VOID(method, ...) \ + { \ + LM_LOCKGUARD(stateLock()); \ + if (nullptr != instance) \ + { \ + instance->method(__VA_ARGS__); \ + return; \ + } \ } #ifdef _MANAGED #define LM_LOCKGUARD(macro_mutex) msclr::lock l(LogManagerLock::lock); #else -#define LM_LOCKGUARD(macro_mutex) \ - std::lock_guard TOKENPASTE2(__guard_, __LINE__) (macro_mutex); +#define LM_LOCKGUARD(macro_mutex) \ + std::lock_guard TOKENPASTE2(__guard_, __LINE__)(macro_mutex); #endif namespace ARIASDK_NS_BEGIN @@ -101,9 +103,11 @@ namespace ARIASDK_NS_BEGIN #if (HAVE_EXCEPTIONS) class LogManagerNotInitializedException : public std::runtime_error { - public: - LogManagerNotInitializedException(const char* message) noexcept - : std::runtime_error(message) { } + public: + LogManagerNotInitializedException(const char* message) noexcept : + std::runtime_error(message) + { + } }; #endif @@ -112,39 +116,39 @@ namespace ARIASDK_NS_BEGIN /// is running in "host" mode and all LogController methods should be accessible to the /// caller. /// - static constexpr const char * HOST_MODE = "hostMode"; + static constexpr const char* HOST_MODE = "hostMode"; /// /// This class is used to manage the Events logging system /// - template class LogManagerBase + template + class LogManagerBase { static_assert(std::is_base_of::value, "ModuleConfiguration must derive from LogConfiguration"); - public: + public: using basetype = LogManagerBase; - protected: - + protected: /// /// LogManagerBase constructor /// - LogManagerBase() {}; + LogManagerBase(){}; /// /// LogManager copy constructor /// - LogManagerBase(const LogManagerBase&) {}; + LogManagerBase(const LogManagerBase&){}; /// /// [not implemented] LogManager assignment operator /// - LogManagerBase& operator=(const LogManagerBase&) {}; + LogManagerBase& operator=(const LogManagerBase&){}; /// /// LogManager destructor /// - virtual ~LogManagerBase() {}; + virtual ~LogManagerBase(){}; #ifndef _MANAGED /// @@ -167,7 +171,7 @@ namespace ARIASDK_NS_BEGIN /// /// Concrete instance for servicing all singleton calls /// - static ILogManager* instance; + static ILogManager* instance; /// /// Debug event source associated with this singleton @@ -181,11 +185,10 @@ namespace ARIASDK_NS_BEGIN static const char* GetPrimaryToken() { ILogConfiguration& config = GetLogConfiguration(); - return (const char *)(config[CFG_STR_PRIMARY_TOKEN]); + return (const char*)(config[CFG_STR_PRIMARY_TOKEN]); } - public: - + public: /// /// Returns this module LogConfiguration /// @@ -201,7 +204,7 @@ namespace ARIASDK_NS_BEGIN /// A logger instance instantiated with the default tenantToken. static ILogger* Initialize() { - return Initialize(std::string {}); + return Initialize(std::string{}); } /// @@ -211,7 +214,7 @@ namespace ARIASDK_NS_BEGIN /// A logger instance instantiated with the tenantToken. inline static ILogger* Initialize(const std::string& tenantToken) { - return Initialize(tenantToken, GetLogConfiguration()); + return Initialize(tenantToken, GetLogConfiguration()); } /// @@ -222,8 +225,7 @@ namespace ARIASDK_NS_BEGIN /// A logger instance instantiated with the tenantToken. static ILogger* Initialize( const std::string& tenantToken, - ILogConfiguration& configuration - ) + ILogConfiguration& configuration) { LM_LOCKGUARD(stateLock()); ILogConfiguration& currentConfig = GetLogConfiguration(); @@ -232,12 +234,12 @@ namespace ARIASDK_NS_BEGIN // Copy alternate configuration into currentConfig if (&configuration != ¤tConfig) { - for (const auto &kv : *configuration) + for (const auto& kv : *configuration) { currentConfig[kv.first.c_str()] = kv.second; } - for (const auto &kv : configuration.GetModules()) + for (const auto& kv : configuration.GetModules()) { currentConfig.AddModule(kv.first.c_str(), kv.second); } @@ -254,7 +256,8 @@ namespace ARIASDK_NS_BEGIN instance->AttachEventSource(GetDebugEventSource()); return instance->GetLogger(currentConfig[CFG_STR_PRIMARY_TOKEN]); } - else { + else + { // TODO: [MG] - decide what to do if someone's doing re-Initialize. // For now Re-initialize works as GetLogger with an alternate token. // We may decide to assert(tenantToken==primaryToken) ... @@ -272,21 +275,26 @@ namespace ARIASDK_NS_BEGIN static status_t FlushAndTeardown() { LM_LOCKGUARD(stateLock()); -#ifdef NO_TEARDOWN // Avoid destroying our ILogManager instance on teardown +#ifdef NO_TEARDOWN // Avoid destroying our ILogManager instance on teardown LM_SAFE_CALL(Flush); LM_SAFE_CALL(UploadNow); return STATUS_SUCCESS; -#else // Less safe approach, but this is in alignment with original v1 behavior - // Side-effect of this is that all ILogger instances get invalidated on FlushAndTeardown - auto callFTD = []() LM_SAFE_CALL(GetLogController()->FlushAndTeardown); - auto result = callFTD(); +#else // Less safe approach, but this is in alignment with original v1 behavior \ + // Side-effect of this is that all ILogger instances get invalidated on FlushAndTeardown if (instance) { + auto controller = instance->GetLogController(); + if (controller != nullptr) + { + controller->FlushAndTeardown(); + } ILogConfiguration& currentConfig = GetLogConfiguration(); - result = LogManagerProvider::Release(currentConfig); + auto result = LogManagerProvider::Release(currentConfig); instance = nullptr; + return result; } - return result; + // already gone + return STATUS_EALREADY; #endif } @@ -307,12 +315,12 @@ namespace ARIASDK_NS_BEGIN LM_LOCKGUARD(stateLock()); if (isHost()) LM_SAFE_CALL(GetLogController()->UploadNow); - return STATUS_EPERM; // Permission denied + return STATUS_EPERM; // Permission denied } /// /// Flush any pending telemetry events in memory to disk to reduce possible data loss as seen necessary. - /// This function can be very expensive so should be used sparingly. OS will block the calling thread + /// This function can be very expensive so should be used sparingly. OS will block the calling thread /// and might flush the global file buffers, i.e. all buffered filesystem data, to disk, which could be /// time consuming. /// @@ -320,7 +328,7 @@ namespace ARIASDK_NS_BEGIN { if (isHost()) LM_SAFE_CALL(GetLogController()->Flush); - return STATUS_EPERM; // Permission denied + return STATUS_EPERM; // Permission denied } /// @@ -331,7 +339,7 @@ namespace ARIASDK_NS_BEGIN { if (isHost()) LM_SAFE_CALL(GetLogController()->PauseTransmission); - return STATUS_EPERM; // Permission denied + return STATUS_EPERM; // Permission denied } /// @@ -341,13 +349,13 @@ namespace ARIASDK_NS_BEGIN { if (isHost()) LM_SAFE_CALL(GetLogController()->ResumeTransmission); - return STATUS_EPERM; // Permission denied + return STATUS_EPERM; // Permission denied } /// /// Sets transmit profile for event transmission to one of the built-in profiles. /// A transmit profile is a collection of hardware and system settings (like network connectivity, power state) - /// based on which to determine how events are to be transmitted. + /// based on which to determine how events are to be transmitted. /// /// Transmit profile /// This function doesn't return any value because it always succeeds. @@ -355,13 +363,13 @@ namespace ARIASDK_NS_BEGIN { if (isHost()) LM_SAFE_CALL(GetLogController()->SetTransmitProfile, profile); - return STATUS_EPERM; // Permission denied + return STATUS_EPERM; // Permission denied } /// /// Sets transmit profile for event transmission. /// A transmit profile is a collection of hardware and system settings (like network connectivity, power state) - /// based on which to determine how events are to be transmitted. + /// based on which to determine how events are to be transmitted. /// /// Transmit profile /// true if profile is successfully applied, false otherwise @@ -369,7 +377,7 @@ namespace ARIASDK_NS_BEGIN { if (isHost()) LM_SAFE_CALL(GetLogController()->SetTransmitProfile, profile); - return STATUS_EPERM; // Permission denied + return STATUS_EPERM; // Permission denied } /// @@ -381,7 +389,7 @@ namespace ARIASDK_NS_BEGIN { if (isHost()) LM_SAFE_CALL(GetLogController()->LoadTransmitProfiles, profiles_json); - return STATUS_EPERM; // Permission denied + return STATUS_EPERM; // Permission denied } /// @@ -391,7 +399,7 @@ namespace ARIASDK_NS_BEGIN { if (isHost()) LM_SAFE_CALL(GetLogController()->ResetTransmitProfiles); - return STATUS_EPERM; // Permission denied + return STATUS_EPERM; // Permission denied } #endif @@ -402,9 +410,9 @@ namespace ARIASDK_NS_BEGIN LM_SAFE_CALL_STR(GetTransmitProfileName); /// - /// Retrieve an ISemanticContext interface through which to specify context information + /// Retrieve an ISemanticContext interface through which to specify context information /// such as device, system, hardware and user information. - /// Context information set via this API will apply to all logger instance unless they + /// Context information set via this API will apply to all logger instance unless they /// are overwritten by individual logger instance. /// /// ISemanticContext interface pointer @@ -413,7 +421,7 @@ namespace ARIASDK_NS_BEGIN /// /// Adds or overrides a property of the custom context for the telemetry logging system. - /// Context information set here applies to events generated by all ILogger instances + /// Context information set here applies to events generated by all ILogger instances /// unless it is overwritten on a particular ILogger instance. /// /// Name of the context property @@ -424,13 +432,13 @@ namespace ARIASDK_NS_BEGIN /// /// Adds or overrides a property of the custom context for the telemetry logging system. - /// Context information set here applies to events generated by all ILogger instances + /// Context information set here applies to events generated by all ILogger instances /// unless it is overwritten on a particular ILogger instance. /// /// Name of the context property /// Value of the context property /// PIIKind of the context with PiiKind_None as the default - static status_t SetContext(const std::string& name, const char *value, PiiKind piiKind = PiiKind_None) + static status_t SetContext(const std::string& name, const char* value, PiiKind piiKind = PiiKind_None) LM_SAFE_CALL(SetContext, name, std::string(value), piiKind); /// @@ -455,7 +463,7 @@ namespace ARIASDK_NS_BEGIN /// /// Name of the property /// 8-bit Integer value of the property - static status_t SetContext(const std::string& name, int8_t value, PiiKind piiKind = PiiKind_None) + static status_t SetContext(const std::string& name, int8_t value, PiiKind piiKind = PiiKind_None) LM_SAFE_CALL(SetContext, name, (int64_t)value, piiKind); /// @@ -482,7 +490,7 @@ namespace ARIASDK_NS_BEGIN /// /// Name of the property /// 8-bit unsigned integer value of the property - static status_t SetContext(const std::string& name, uint8_t value, PiiKind piiKind = PiiKind_None) + static status_t SetContext(const std::string& name, uint8_t value, PiiKind piiKind = PiiKind_None) LM_SAFE_CALL(SetContext, name, (int64_t)value, piiKind); /// @@ -588,7 +596,7 @@ namespace ARIASDK_NS_BEGIN /// /// Add Debug callback /// - static void AddEventListener(DebugEventType type, DebugEventListener &listener) + static void AddEventListener(DebugEventType type, DebugEventListener& listener) { GetDebugEventSource().AddEventListener(type, listener); } @@ -596,7 +604,7 @@ namespace ARIASDK_NS_BEGIN /// /// Remove Debug callback /// - static void RemoveEventListener(DebugEventType type, DebugEventListener &listener) + static void RemoveEventListener(DebugEventType type, DebugEventListener& listener) { GetDebugEventSource().RemoveEventListener(type, listener); } @@ -683,15 +691,17 @@ namespace ARIASDK_NS_BEGIN // that causes improper global static templated member initialization: // https://developercommunity.visualstudio.com/content/problem/134886/initialization-of-template-static-variable-wrong.html // -#define DEFINE_LOGMANAGER(LogManagerClass, LogConfigurationClass) \ - ILogManager* LogManagerClass::instance = nullptr; +#define DEFINE_LOGMANAGER(LogManagerClass, LogConfigurationClass) \ + ILogManager* LogManagerClass::instance = nullptr; #else // ISO C++ -compliant declaration -#define DEFINE_LOGMANAGER(LogManagerClass, LogConfigurationClass) \ - template<> ILogManager* LogManagerBase::instance {}; +#define DEFINE_LOGMANAGER(LogManagerClass, LogConfigurationClass) \ + template <> \ + ILogManager* LogManagerBase::instance{}; #endif -} ARIASDK_NS_END +} +ARIASDK_NS_END #pragma clang diagnostic pop diff --git a/lib/pal/PAL.hpp b/lib/pal/PAL.hpp index e1bdd9850..fbdba3021 100644 --- a/lib/pal/PAL.hpp +++ b/lib/pal/PAL.hpp @@ -2,9 +2,9 @@ #ifndef PAL_HPP #define PAL_HPP -#include "mat/config.h" -#include "Version.hpp" #include "DebugTrace.hpp" +#include "Version.hpp" +#include "mat/config.h" #ifdef HAVE_MAT_EXP #define ECS_SUPP "ECS" @@ -12,9 +12,9 @@ #define ECS_SUPP "No" #endif -#include "SystemInformationImpl.hpp" -#include "NetworkInformationImpl.hpp" #include "DeviceInformationImpl.hpp" +#include "NetworkInformationImpl.hpp" +#include "SystemInformationImpl.hpp" #include "ISemanticContext.hpp" @@ -26,8 +26,8 @@ #ifndef WIN32_LEAN_AND_MEAN #define WIN32_LEAN_AND_MEAN #endif -#include #include +#include #else /* POSIX */ #define PATH_SEPARATOR_CHAR '/' @@ -39,37 +39,41 @@ #include #include +#include +#include +#include +#include #include #include #include +#include #include #include -#include -#include -#include -#include -#include -#include #include +#include +#include "WorkerThread.hpp" #include "api/IRuntimeConfig.hpp" #include "typename.hpp" -#include "WorkerThread.hpp" namespace ARIASDK_NS_BEGIN { void print_backtrace(); -} ARIASDK_NS_END +} +ARIASDK_NS_END namespace PAL_NS_BEGIN { class INetworkInformation; class IDeviceInformation; class ISystemInformation; + class PALTest; class PlatformAbstractionLayer { - public: + friend class PALTest; + + public: void sleep(unsigned delayMs) const noexcept; const std::string& getSdkVersion() const; @@ -93,7 +97,7 @@ namespace PAL_NS_BEGIN void initialize(IRuntimeConfig& configuration); void shutdown(); - + INetworkInformation* GetNetworkInformation() const noexcept; IDeviceInformation* GetDeviceInformation() const noexcept; ISystemInformation* GetSystemInformation() const noexcept; @@ -104,8 +108,8 @@ namespace PAL_NS_BEGIN MATSDK_LOG_DECL_COMPONENT_CLASS(); - private: - volatile std::atomic m_palStarted { 0 }; + private: + volatile std::atomic m_palStarted{0}; std::shared_ptr m_taskDispatcher; ISystemInformation* m_SystemInformation; INetworkInformation* m_NetworkInformation; @@ -233,6 +237,7 @@ namespace PAL_NS_BEGIN return GetPAL().RegisterIkeyWithWindowsTelemetry(ikeyin, storageSize, uploadQuotaSize); } -} PAL_NS_END +} +PAL_NS_END #endif diff --git a/tests/functests/BasicFuncTests.cpp b/tests/functests/BasicFuncTests.cpp index 65e47245d..3a59920d6 100644 --- a/tests/functests/BasicFuncTests.cpp +++ b/tests/functests/BasicFuncTests.cpp @@ -3,26 +3,26 @@ #ifdef HAVE_MAT_DEFAULT_HTTP_CLIENT #ifndef WIN32_LEAN_AND_MEAN -#define WIN32_LEAN_AND_MEAN // Exclude rarely-used stuff from Windows headers +#define WIN32_LEAN_AND_MEAN // Exclude rarely-used stuff from Windows headers #endif +#include "api/LogManagerImpl.hpp" #include "common/Common.hpp" #include "common/HttpServer.hpp" #include "utils/Utils.hpp" -#include "api/LogManagerImpl.hpp" +#include "LogManager.hpp" #include "bond/All.hpp" -#include "bond/generated/CsProtocol_types.hpp" #include "bond/generated/CsProtocol_readers.hpp" -#include "LogManager.hpp" +#include "bond/generated/CsProtocol_types.hpp" #include "CompliantByDefaultFilterApi.hpp" -#include -#include #include +#include +#include #include +#include #include -#include #include #include "PayloadDecoder.hpp" @@ -32,43 +32,97 @@ using namespace MAT; // LOGMANAGER_INSTANCE +namespace PAL_NS_BEGIN +{ + class PALTest + { + public: + + static long GetPalRefCount() + { + return PAL::GetPAL().m_palStarted.load(); + }; + + static std::shared_ptr GetTaskDispatcher() + { + return PAL::GetPAL().m_taskDispatcher; + }; + + static ISystemInformation* GetSystemInformation() + { + return PAL::GetPAL().m_SystemInformation; + } + + static INetworkInformation* GetNetworkInformation() + { + return PAL::GetPAL().m_NetworkInformation; + } + + static IDeviceInformation* GetDeviceInformation() + { + return PAL::GetPAL().m_DeviceInformation; + } + }; +} +PAL_NS_END; + +namespace ARIASDK_NS_BEGIN +{ + class ModuleA : public ILogConfiguration + { + }; + class LogManagerA : public LogManagerBase + { + }; + class ModuleB : public ILogConfiguration + { + }; + class LogManagerB : public LogManagerBase + { + }; + // Two distinct LogManagerX 'singelton' instances + DEFINE_LOGMANAGER(LogManagerB, ModuleB); + DEFINE_LOGMANAGER(LogManagerA, ModuleA); +} +ARIASDK_NS_END + char const* const TEST_STORAGE_FILENAME = "BasicFuncTests.db"; // 1DSCppSdktest sandbox key -#define TEST_TOKEN "7c8b1796cbc44bd5a03803c01c2b9d61-b6e370dd-28d9-4a52-9556-762543cf7aa7-6991" +#define TEST_TOKEN "7c8b1796cbc44bd5a03803c01c2b9d61-b6e370dd-28d9-4a52-9556-762543cf7aa7-6991" -#define KILLED_TOKEN "deadbeefdeadbeefdeadbeefdeadbeef-c2d379e0-4408-4325-9b4d-2a7d78131e14-7322" -#define HTTP_PORT 19000 +#define KILLED_TOKEN "deadbeefdeadbeefdeadbeefdeadbeef-c2d379e0-4408-4325-9b4d-2a7d78131e14-7322" +#define HTTP_PORT 19000 #undef LOCKGUARD -#define LOCKGUARD(macro_mutex) std::lock_guard TOKENPASTE2(__guard_, __LINE__) (macro_mutex); +#define LOCKGUARD(macro_mutex) std::lock_guard TOKENPASTE2(__guard_, __LINE__)(macro_mutex); class HttpPostListener : public DebugEventListener { -public: - virtual void OnDebugEvent(DebugEvent &evt) + public: + virtual void OnDebugEvent(DebugEvent& evt) { static unsigned seq = 0; switch (evt.type) { case EVT_HTTP_OK: - { - seq++; - std::string out; - std::vector reqBody((unsigned char *)evt.data, (unsigned char *)(evt.data) + evt.size); - MAT::exporters::DecodeRequest(reqBody, out, false); - printf(">>>> REQUEST [%u]:%s\n", seq, out.c_str()); - } - break; + { + seq++; + std::string out; + std::vector reqBody((unsigned char*)evt.data, (unsigned char*)(evt.data) + evt.size); + MAT::exporters::DecodeRequest(reqBody, out, false); + printf(">>>> REQUEST [%u]:%s\n", seq, out.c_str()); + } + break; default: break; }; }; }; class BasicFuncTests : public ::testing::Test, - public HttpServer::Callback + public HttpServer::Callback { -protected: - std::mutex mtx_requests; + protected: + std::mutex mtx_requests; std::vector receivedRequests; std::string serverAddress; HttpServer server; @@ -81,12 +135,11 @@ class BasicFuncTests : public ::testing::Test, std::condition_variable cv_gotEvents; std::mutex cv_m; -public: - BasicFuncTests() : - isSetup(false) , - isRunning(false) - {}; + public: + BasicFuncTests() : + isSetup(false), + isRunning(false){}; virtual void SetUp() override { @@ -102,7 +155,7 @@ class BasicFuncTests : public ::testing::Test, server.addHandler("/simple/", *this); server.addHandler("/slow/", *this); server.addHandler("/503/", *this); - server.setKeepalive(false); // This test doesn't work well with keep-alive enabled + server.setKeepalive(false); // This test doesn't work well with keep-alive enabled server.start(); isRunning = true; } @@ -138,20 +191,20 @@ class BasicFuncTests : public ::testing::Test, configuration[CFG_INT_RAM_QUEUE_SIZE] = 4096 * 20; configuration[CFG_STR_CACHE_FILE_PATH] = TEST_STORAGE_FILENAME; - configuration[CFG_INT_MAX_TEARDOWN_TIME] = 2; // 2 seconds wait on shutdown + configuration[CFG_INT_MAX_TEARDOWN_TIME] = 2; // 2 seconds wait on shutdown configuration[CFG_STR_COLLECTOR_URL] = serverAddress.c_str(); - configuration["http"]["compress"] = false; // disable compression for now - configuration["stats"]["interval"] = 30 * 60; // 30 mins + configuration["http"]["compress"] = false; // disable compression for now + configuration["stats"]["interval"] = 30 * 60; // 30 mins configuration["name"] = __FILE__; configuration["version"] = "1.0.0"; - configuration["config"] = { { "host", __FILE__ } }; // Host instance + configuration["config"] = {{"host", __FILE__}}; // Host instance LogManager::Initialize(TEST_TOKEN, configuration); - LogManager::SetLevelFilter(DIAG_LEVEL_DEFAULT, { DIAG_LEVEL_DEFAULT_MIN, DIAG_LEVEL_DEFAULT_MAX }); + LogManager::SetLevelFilter(DIAG_LEVEL_DEFAULT, {DIAG_LEVEL_DEFAULT_MIN, DIAG_LEVEL_DEFAULT_MAX}); LogManager::ResumeTransmission(); - logger = LogManager::GetLogger(TEST_TOKEN, "source1"); + logger = LogManager::GetLogger(TEST_TOKEN, "source1"); logger2 = LogManager::GetLogger(TEST_TOKEN, "source2"); } @@ -162,17 +215,19 @@ class BasicFuncTests : public ::testing::Test, virtual int onHttpRequest(HttpServer::Request const& request, HttpServer::Response& response) override { - if (request.uri.compare(0, 5, "/503/") == 0) { + if (request.uri.compare(0, 5, "/503/") == 0) + { return 503; } - if (request.uri.compare(0, 6, "/slow/") == 0) { + if (request.uri.compare(0, 6, "/slow/") == 0) + { PAL::sleep(static_cast(request.content.size() / DELAY_FACTOR_FOR_SERVER)); } { LOCKGUARD(mtx_requests); - receivedRequests.push_back(request); + receivedRequests.push_back(request); } response.headers["Content-Type"] = "text/plain"; @@ -196,7 +251,7 @@ class BasicFuncTests : public ::testing::Test, unsigned receivedEvents = 0; auto start = PAL::getUtcSystemTimeMs(); size_t lastIdx = 0; - while ( ((PAL::getUtcSystemTimeMs()-start)<(1000* timeOutSec)) && (receivedEvents!=expected_count) ) + while (((PAL::getUtcSystemTimeMs() - start) < (1000 * timeOutSec)) && (receivedEvents != expected_count)) { /* Give time for our friendly HTTP server thread to processs incoming request */ std::this_thread::yield(); @@ -211,7 +266,7 @@ class BasicFuncTests : public ::testing::Test, { auto request = receivedRequests.at(index); auto payload = decodeRequest(request, false); - receivedEvents+= (unsigned)payload.size(); + receivedEvents += (unsigned)payload.size(); } lastIdx = size; } @@ -266,7 +321,6 @@ class BasicFuncTests : public ::testing::Test, EXPECT_THAT(bond_lite::Deserialize(reader, result), true); data += index - 1; vector.push_back(result); - } return vector; @@ -319,7 +373,7 @@ class BasicFuncTests : public ::testing::Test, } case ::CsProtocol::ValueGuid: { - uint8_t guid_bytes[16] = { 0 }; + uint8_t guid_bytes[16] = {0}; prop.second.as_guid.to_bytes(guid_bytes); std::vector guid = std::vector(guid_bytes, guid_bytes + sizeof(guid_bytes) / sizeof(guid_bytes[0])); @@ -382,7 +436,7 @@ class BasicFuncTests : public ::testing::Test, EXPECT_THAT(vectror.size(), prop.second.as_guidArray->size()); for (size_t index = 0; index < prop.second.as_guidArray->size(); index++) { - uint8_t guid_bytes[16] = { 0 }; + uint8_t guid_bytes[16] = {0}; prop.second.as_guidArray->at(index).to_bytes(guid_bytes); std::vector guid = std::vector(guid_bytes, guid_bytes + sizeof(guid_bytes) / sizeof(guid_bytes[0])); @@ -433,11 +487,11 @@ class BasicFuncTests : public ::testing::Test, std::vector result; if (receivedRequests.size()) { - for (auto &request : receivedRequests) + for (auto& request : receivedRequests) { // TODO: [MG] - add compression support auto payload = decodeRequest(request, false); - for (auto &record : payload) + for (auto& record : payload) { result.push_back(std::move(record)); } @@ -453,11 +507,11 @@ class BasicFuncTests : public ::testing::Test, result.name = ""; if (receivedRequests.size()) { - for (auto &request : receivedRequests) + for (auto& request : receivedRequests) { // TODO: [MG] - add compression support auto payload = decodeRequest(request, false); - for (auto &record : payload) + for (auto& record : payload) { if (record.name == name) { @@ -471,7 +525,6 @@ class BasicFuncTests : public ::testing::Test, } }; - TEST_F(BasicFuncTests, doNothing) { } @@ -484,7 +537,7 @@ TEST_F(BasicFuncTests, sendOneEvent_immediatelyStop) event.SetProperty("property", "value"); logger->LogEvent(event); FlushAndTeardown(); - EXPECT_GE(receivedRequests.size(), (size_t)1); // at least 1 HTTP request with customer payload and stats + EXPECT_GE(receivedRequests.size(), (size_t)1); // at least 1 HTTP request with customer payload and stats } TEST_F(BasicFuncTests, sendNoPriorityEvents) @@ -511,10 +564,9 @@ TEST_F(BasicFuncTests, sendNoPriorityEvents) if (receivedRequests.size() >= 1) { - verifyEvent(event, find("first_event")); + verifyEvent(event, find("first_event")); verifyEvent(event2, find("second_event")); } - } TEST_F(BasicFuncTests, sendSamePriorityNormalEvents) @@ -552,7 +604,7 @@ TEST_F(BasicFuncTests, sendSamePriorityNormalEvents) logger->LogEvent(event2); waitForEvents(2, 3); - for (const auto &evt : { event, event2 }) + for (const auto& evt : {event, event2}) { verifyEvent(evt, find(evt.GetName())); } @@ -588,7 +640,6 @@ TEST_F(BasicFuncTests, sendDifferentPriorityEvents) std::fill(gvector.begin() + 3, gvector.end() - 2, GUID_t("00000000-0000-0000-0000-000000000000")); event.SetProperty("property4", gvector); - logger->LogEvent(event); EventProperties event2("second_event"); @@ -598,13 +649,12 @@ TEST_F(BasicFuncTests, sendDifferentPriorityEvents) event2.SetProperty("pii_property", "pii_value", PiiKind_Identity); event2.SetProperty("cc_property", "cc_value", CustomerContentKind_GenericData); - logger->LogEvent(event2); LogManager::UploadNow(); waitForEvents(1, 3); - for (const auto &evt : { event, event2 }) + for (const auto& evt : {event, event2}) { verifyEvent(evt, find(evt.GetName())); } @@ -649,13 +699,12 @@ TEST_F(BasicFuncTests, sendMultipleTenantsTogether) LogManager::UploadNow(); waitForEvents(1, 3); - for (const auto &evt : { event1, event2 }) + for (const auto& evt : {event1, event2}) { verifyEvent(evt, find(evt.GetName())); } FlushAndTeardown(); - } TEST_F(BasicFuncTests, configDecorations) @@ -678,7 +727,7 @@ TEST_F(BasicFuncTests, configDecorations) LogManager::UploadNow(); waitForEvents(2, 5); - for (const auto &evt : { event1, event2, event3, event4 }) + for (const auto& evt : {event1, event2, event3, event4}) { verifyEvent(evt, find(evt.GetName())); } @@ -715,7 +764,7 @@ TEST_F(BasicFuncTests, restartRecoversEventsFromStorage) LogManager::UploadNow(); // 1st request for realtime event - waitForEvents(3, 7); // start, first_event, second_event, ongoing, stop, start, fooEvent + waitForEvents(3, 7); // start, first_event, second_event, ongoing, stop, start, fooEvent EXPECT_GE(receivedRequests.size(), (size_t)1); if (receivedRequests.size() != 0) { @@ -736,7 +785,7 @@ TEST_F(BasicFuncTests, restartRecoversEventsFromStorage) */ } -#if 0 // FIXME: 1445871 [v3][1DS] Offline storage size may exceed configured limit +#if 0 // FIXME: 1445871 [v3][1DS] Offline storage size may exceed configured limit TEST_F(BasicFuncTests, storageFileSizeDoesntExceedConfiguredSize) { CleanStorage(); @@ -818,18 +867,18 @@ TEST_F(BasicFuncTests, sendMetaStatsOnStart) FlushAndTeardown(); auto r1 = records(); - ASSERT_EQ(r1.size(), size_t { 0 }); + ASSERT_EQ(r1.size(), size_t{0}); // Check Initialize(); - LogManager::ResumeTransmission(); // ? + LogManager::ResumeTransmission(); // ? LogManager::UploadNow(); PAL::sleep(2000); auto r2 = records(); - ASSERT_GE(r2.size(), (size_t)4); // (start + stop) + (2 events + start) + ASSERT_GE(r2.size(), (size_t)4); // (start + stop) + (2 events + start) - for (const auto &evt : { event1, event2 }) + for (const auto& evt : {event1, event2}) { verifyEvent(evt, find(evt.GetName())); } @@ -841,7 +890,7 @@ TEST_F(BasicFuncTests, DiagLevelRequiredOnly_OneEventWithoutLevelOneWithButNotAl { CleanStorage(); Initialize(); - LogManager::SetLevelFilter(DIAG_LEVEL_OPTIONAL, { DIAG_LEVEL_REQUIRED }); + LogManager::SetLevelFilter(DIAG_LEVEL_OPTIONAL, {DIAG_LEVEL_REQUIRED}); EventProperties eventWithoutLevel("EventWithoutLevel"); logger->LogEvent(eventWithoutLevel); @@ -856,7 +905,7 @@ TEST_F(BasicFuncTests, DiagLevelRequiredOnly_OneEventWithoutLevelOneWithButNotAl LogManager::UploadNow(); waitForEvents(1 /*timeout*/, 2 /*expected count*/); // Start and EventWithAllowedLevel - ASSERT_EQ(records().size(), static_cast(2)); // Start and EventWithAllowedLevel + ASSERT_EQ(records().size(), static_cast(2)); // Start and EventWithAllowedLevel verifyEvent(eventWithAllowedLevel, find(eventWithAllowedLevel.GetName())); @@ -890,34 +939,34 @@ TEST_F(BasicFuncTests, DiagLevelRequiredOnly_SendTwoEventsUpdateAllowedLevelsSen CleanStorage(); Initialize(); - LogManager::SetLevelFilter(DIAG_LEVEL_OPTIONAL, { DIAG_LEVEL_REQUIRED }); + LogManager::SetLevelFilter(DIAG_LEVEL_OPTIONAL, {DIAG_LEVEL_REQUIRED}); SendEventWithOptionalThenRequired(logger); - LogManager::SetLevelFilter(DIAG_LEVEL_OPTIONAL, { DIAG_LEVEL_OPTIONAL, DIAG_LEVEL_REQUIRED }); + LogManager::SetLevelFilter(DIAG_LEVEL_OPTIONAL, {DIAG_LEVEL_OPTIONAL, DIAG_LEVEL_REQUIRED}); SendEventWithOptionalThenRequired(logger); LogManager::UploadNow(); - waitForEvents(2 /*timeout*/, 4 /*expected count*/); // Start and EventWithAllowedLevel + waitForEvents(2 /*timeout*/, 4 /*expected count*/); // Start and EventWithAllowedLevel auto sentRecords = records(); - ASSERT_EQ(sentRecords.size(), static_cast(4)); // Start and EventWithAllowedLevel - ASSERT_EQ(GetEventsWithName("EventWithOptionalLevel", sentRecords).size(), size_t{ 1 }); - ASSERT_EQ(GetEventsWithName("EventWithRequiredLevel", sentRecords).size(), size_t{ 2 }); + ASSERT_EQ(sentRecords.size(), static_cast(4)); // Start and EventWithAllowedLevel + ASSERT_EQ(GetEventsWithName("EventWithOptionalLevel", sentRecords).size(), size_t{1}); + ASSERT_EQ(GetEventsWithName("EventWithRequiredLevel", sentRecords).size(), size_t{2}); FlushAndTeardown(); } class RequestMonitor : public DebugEventListener { - const size_t IDX_OK = 0; - const size_t IDX_ERR = 1; + const size_t IDX_OK = 0; + const size_t IDX_ERR = 1; const size_t IDX_ABRT = 2; std::atomic counts[3]; -public: - - RequestMonitor() : DebugEventListener() + public: + RequestMonitor() : + DebugEventListener() { reset(); } @@ -929,7 +978,7 @@ class RequestMonitor : public DebugEventListener counts[IDX_ABRT] = 0; } - virtual void OnDebugEvent(DebugEvent &evt) + virtual void OnDebugEvent(DebugEvent& evt) { switch (evt.type) { @@ -959,16 +1008,17 @@ class RequestMonitor : public DebugEventListener } }; -class KillSwitchListener : public DebugEventListener { -public : - std::atomic numLogged; - std::atomic numSent; - std::atomic numDropped; - std::atomic numReject; - std::atomic numHttpError; - std::atomic numHttpOK; - std::atomic numHttpFailure; - std::atomic numCached; +class KillSwitchListener : public DebugEventListener +{ + public: + std::atomic numLogged; + std::atomic numSent; + std::atomic numDropped; + std::atomic numReject; + std::atomic numHttpError; + std::atomic numHttpOK; + std::atomic numHttpFailure; + std::atomic numCached; KillSwitchListener() : numLogged(0), @@ -978,45 +1028,48 @@ public : numHttpError(0), numHttpOK(0), numHttpFailure(0), - numCached(0) + numCached(0) { } - virtual void OnDebugEvent(DebugEvent &evt) { - switch (evt.type) { - case EVT_LOG_SESSION: - numLogged++; - break; - case EVT_REJECTED: - numReject++; - break; - case EVT_ADDED: - break; - /* Event counts below would never overflow the size of unsigned int */ - case EVT_CACHED: - numCached += (unsigned int)evt.param1; - break; - case EVT_DROPPED: - numDropped += (unsigned int)evt.param1; - break; - case EVT_SENT: - numSent += (unsigned int)evt.param1; - break; - case EVT_HTTP_FAILURE: - numHttpFailure++; - break; - case EVT_HTTP_ERROR: - numHttpError++; - break; - case EVT_HTTP_OK: - numHttpOK++; - break; - case EVT_UNKNOWN: - default: + virtual void OnDebugEvent(DebugEvent& evt) + { + switch (evt.type) + { + case EVT_LOG_SESSION: + numLogged++; + break; + case EVT_REJECTED: + numReject++; + break; + case EVT_ADDED: + break; + /* Event counts below would never overflow the size of unsigned int */ + case EVT_CACHED: + numCached += (unsigned int)evt.param1; + break; + case EVT_DROPPED: + numDropped += (unsigned int)evt.param1; + break; + case EVT_SENT: + numSent += (unsigned int)evt.param1; + break; + case EVT_HTTP_FAILURE: + numHttpFailure++; + break; + case EVT_HTTP_ERROR: + numHttpError++; + break; + case EVT_HTTP_OK: + numHttpOK++; + break; + case EVT_UNKNOWN: + default: break; }; } - void printStats(){ + void printStats() + { std::cerr << "[ ] numLogged = " << numLogged << std::endl; std::cerr << "[ ] numSent = " << numSent << std::endl; std::cerr << "[ ] numDropped = " << numDropped << std::endl; @@ -1028,7 +1081,8 @@ public : } }; -void addListeners(DebugEventListener &listener) { +void addListeners(DebugEventListener& listener) +{ LogManager::AddEventListener(DebugEventType::EVT_LOG_SESSION, listener); LogManager::AddEventListener(DebugEventType::EVT_REJECTED, listener); LogManager::AddEventListener(DebugEventType::EVT_SENT, listener); @@ -1039,7 +1093,8 @@ void addListeners(DebugEventListener &listener) { LogManager::AddEventListener(DebugEventType::EVT_CACHED, listener); } -void removeListeners(DebugEventListener &listener) { +void removeListeners(DebugEventListener& listener) +{ LogManager::RemoveEventListener(DebugEventType::EVT_LOG_SESSION, listener); LogManager::RemoveEventListener(DebugEventType::EVT_REJECTED, listener); LogManager::RemoveEventListener(DebugEventType::EVT_SENT, listener); @@ -1062,14 +1117,14 @@ TEST_F(BasicFuncTests, killSwitchWorks) configuration[CFG_INT_RAM_QUEUE_SIZE] = 4096 * 20; configuration[CFG_STR_CACHE_FILE_PATH] = TEST_STORAGE_FILENAME; - configuration[CFG_INT_MAX_TEARDOWN_TIME] = 2; // 2 seconds wait on shutdown + configuration[CFG_INT_MAX_TEARDOWN_TIME] = 2; // 2 seconds wait on shutdown configuration[CFG_STR_COLLECTOR_URL] = serverAddress.c_str(); - configuration["http"]["compress"] = false; // disable compression for now - configuration["stats"]["interval"] = 30 * 60; // 30 mins + configuration["http"]["compress"] = false; // disable compression for now + configuration["stats"]["interval"] = 30 * 60; // 30 mins configuration["name"] = __FILE__; configuration["version"] = "1.0.0"; - configuration["config"] = { { "host", __FILE__ } }; // Host instance + configuration["config"] = {{"host", __FILE__}}; // Host instance // set the killed token on the server server.setKilledToken(KILLED_TOKEN, 6384); @@ -1077,13 +1132,15 @@ TEST_F(BasicFuncTests, killSwitchWorks) addListeners(listener); // Log 100 events from valid and invalid 4 times int repetitions = 4; - for (int i = 0; i < repetitions; i++) { + for (int i = 0; i < repetitions; i++) + { // Initialize the logger for the valid token and log 100 events LogManager::Initialize(TEST_TOKEN, configuration); LogManager::ResumeTransmission(); auto myLogger = LogManager::GetLogger(TEST_TOKEN, "killed"); int numIterations = 100; - while (numIterations--) { + while (numIterations--) + { EventProperties event1("fooEvent"); event1.SetProperty("property", "value"); myLogger->LogEvent(event1); @@ -1093,7 +1150,8 @@ TEST_F(BasicFuncTests, killSwitchWorks) LogManager::ResumeTransmission(); myLogger = LogManager::GetLogger(KILLED_TOKEN, "killed"); numIterations = 100; - while (numIterations--) { + while (numIterations--) + { EventProperties event2("failEvent"); event2.SetProperty("property", "value"); myLogger->LogEvent(event2); @@ -1108,7 +1166,8 @@ TEST_F(BasicFuncTests, killSwitchWorks) LogManager::ResumeTransmission(); auto myLogger = LogManager::GetLogger(TEST_TOKEN, "killed"); int numIterations = 100; - while (numIterations--) { + while (numIterations--) + { EventProperties event1("fooEvent"); event1.SetProperty("property", "value"); myLogger->LogEvent(event1); @@ -1118,13 +1177,14 @@ TEST_F(BasicFuncTests, killSwitchWorks) LogManager::ResumeTransmission(); myLogger = LogManager::GetLogger(KILLED_TOKEN, "killed"); numIterations = 100; - while (numIterations--) { + while (numIterations--) + { EventProperties event2("failEvent"); event2.SetProperty("property", "value"); myLogger->LogEvent(event2); } // Expect all events to be dropped - EXPECT_EQ(uint32_t { 100 }, listener.numDropped); + EXPECT_EQ(uint32_t{100}, listener.numDropped); LogManager::FlushAndTeardown(); listener.printStats(); @@ -1132,7 +1192,7 @@ TEST_F(BasicFuncTests, killSwitchWorks) server.clearKilledTokens(); } -TEST_F(BasicFuncTests, killIsTemporary) +TEST_F(BasicFuncTests, killIsTemporary) { CleanStorage(); // Create the configuration to send to fake server @@ -1144,28 +1204,30 @@ TEST_F(BasicFuncTests, killIsTemporary) configuration[CFG_INT_RAM_QUEUE_SIZE] = 4096 * 20; configuration[CFG_STR_CACHE_FILE_PATH] = TEST_STORAGE_FILENAME; - configuration[CFG_INT_MAX_TEARDOWN_TIME] = 2; // 2 seconds wait on shutdown + configuration[CFG_INT_MAX_TEARDOWN_TIME] = 2; // 2 seconds wait on shutdown configuration[CFG_STR_COLLECTOR_URL] = serverAddress.c_str(); - configuration["http"]["compress"] = false; // disable compression for now - configuration["stats"]["interval"] = 30 * 60; // 30 mins + configuration["http"]["compress"] = false; // disable compression for now + configuration["stats"]["interval"] = 30 * 60; // 30 mins configuration["name"] = __FILE__; configuration["version"] = "1.0.0"; - configuration["config"] = { { "host", __FILE__ } }; // Host instance - + configuration["config"] = {{"host", __FILE__}}; // Host instance + // set the killed token on the server server.setKilledToken(KILLED_TOKEN, 10); KillSwitchListener listener; addListeners(listener); // Log 100 events from valid and invalid 4 times int repetitions = 4; - for (int i = 0; i < repetitions; i++) { + for (int i = 0; i < repetitions; i++) + { // Initialize the logger for the valid token and log 100 events LogManager::Initialize(TEST_TOKEN, configuration); LogManager::ResumeTransmission(); auto myLogger = LogManager::GetLogger(TEST_TOKEN, "killed"); int numIterations = 100; - while (numIterations--) { + while (numIterations--) + { EventProperties event1("fooEvent"); event1.SetProperty("property", "value"); myLogger->LogEvent(event1); @@ -1175,7 +1237,8 @@ TEST_F(BasicFuncTests, killIsTemporary) LogManager::ResumeTransmission(); myLogger = LogManager::GetLogger(KILLED_TOKEN, "killed"); numIterations = 100; - while (numIterations--) { + while (numIterations--) + { EventProperties event2("failEvent"); event2.SetProperty("property", "value"); myLogger->LogEvent(event2); @@ -1192,7 +1255,8 @@ TEST_F(BasicFuncTests, killIsTemporary) LogManager::ResumeTransmission(); auto myLogger = LogManager::GetLogger(TEST_TOKEN, "killed"); int numIterations = 100; - while (numIterations--) { + while (numIterations--) + { EventProperties event1("fooEvent"); event1.SetProperty("property", "value"); myLogger->LogEvent(event1); @@ -1202,13 +1266,14 @@ TEST_F(BasicFuncTests, killIsTemporary) LogManager::ResumeTransmission(); myLogger = LogManager::GetLogger(KILLED_TOKEN, "killed"); numIterations = 100; - while (numIterations--) { + while (numIterations--) + { EventProperties event2("failEvent"); event2.SetProperty("property", "value"); myLogger->LogEvent(event2); } // Expect to 0 events to be dropped - EXPECT_EQ(uint32_t { 0 }, listener.numDropped); + EXPECT_EQ(uint32_t{0}, listener.numDropped); LogManager::FlushAndTeardown(); listener.printStats(); @@ -1226,8 +1291,7 @@ TEST_F(BasicFuncTests, sendManyRequestsAndCancel) auto eventsList = { DebugEventType::EVT_HTTP_OK, DebugEventType::EVT_HTTP_ERROR, - DebugEventType::EVT_HTTP_FAILURE - }; + DebugEventType::EVT_HTTP_FAILURE}; // Add event listeners for (auto evt : eventsList) { @@ -1236,7 +1300,7 @@ TEST_F(BasicFuncTests, sendManyRequestsAndCancel) for (size_t i = 0; i < 20; i++) { - auto &configuration = LogManager::GetLogConfiguration(); + auto& configuration = LogManager::GetLogConfiguration(); configuration[CFG_INT_RAM_QUEUE_SIZE] = 4096 * 20; configuration[CFG_STR_CACHE_FILE_PATH] = TEST_STORAGE_FILENAME; configuration["http"]["compress"] = true; @@ -1279,9 +1343,107 @@ TEST_F(BasicFuncTests, sendManyRequestsAndCancel) LogManager::RemoveEventListener(evt, listener); } } + +#define MAX_TEST_RETRIES 10 + +TEST_F(BasicFuncTests, raceBetweenUploadAndShutdownMultipleLogManagers) +{ + CleanStorage(); + + RequestMonitor listener; + auto eventsList = { + DebugEventType::EVT_HTTP_OK, + DebugEventType::EVT_HTTP_ERROR, + DebugEventType::EVT_HTTP_FAILURE}; + // Add event listeners + for (auto evt : eventsList) + { + LogManagerA::AddEventListener(evt, listener); + LogManagerB::AddEventListener(evt, listener); + }; + + // string values in ILogConfiguration must stay immutable for the duration of the run + { // LogManager A + auto& configuration = LogManagerA::GetLogConfiguration(); + configuration[CFG_INT_RAM_QUEUE_SIZE] = 4096 * 20; + configuration[CFG_STR_CACHE_FILE_PATH] = TEST_STORAGE_FILENAME; + configuration["http"]["compress"] = true; + configuration[CFG_STR_COLLECTOR_URL] = COLLECTOR_URL_PROD; + configuration[CFG_INT_MAX_TEARDOWN_TIME] = 1; + configuration[CFG_INT_TRACE_LEVEL_MASK] = 0; + configuration["name"] = "LogManagerA"; + configuration["version"] = "1.0.0"; + configuration["config"]["host"] = "LogManagerA"; + } + { // LogManager B + auto& configuration = LogManagerB::GetLogConfiguration(); + configuration[CFG_INT_RAM_QUEUE_SIZE] = 4096 * 20; + configuration[CFG_STR_CACHE_FILE_PATH] = TEST_STORAGE_FILENAME; + configuration["http"]["compress"] = true; + configuration[CFG_STR_COLLECTOR_URL] = COLLECTOR_URL_PROD; + configuration[CFG_INT_MAX_TEARDOWN_TIME] = 1; + configuration[CFG_INT_TRACE_LEVEL_MASK] = 0; + configuration["name"] = "LogManagerB"; + configuration["version"] = "1.0.0"; + configuration["config"]["host"] = "LogManagerB"; + } + + std::atomic testRunning(true); + auto t = std::thread([&]() { + while (testRunning) + { + // Abuse LogManagerA and LogManagerB badly. + // Both may or may not have a valid implementation instance. + // PAL could be dead while this abuse is happening. + LogManagerA::UploadNow(); + LogManagerB::UploadNow(); + } + }); + + for (size_t i = 0; i < MAX_TEST_RETRIES; i++) + { + auto loggerA = LogManagerA::Initialize(TEST_TOKEN); + EXPECT_EQ(PAL::PALTest::GetPalRefCount(), 1); + + auto loggerB = LogManagerB::Initialize(TEST_TOKEN); + EXPECT_EQ(PAL::PALTest::GetPalRefCount(), 2); + + EventProperties evtCritical("BasicFuncTests.stress_test_critical_A"); + evtCritical.SetPriority(EventPriority_Immediate); + evtCritical.SetPolicyBitFlags(MICROSOFT_EVENTTAG_CORE_DATA | MICROSOFT_EVENTTAG_REALTIME_LATENCY | MICROSOFT_KEYWORD_CRITICAL_DATA); + loggerA->LogEvent("BasicFuncTests.stress_test_A"); + LogManagerA::UploadNow(); + + loggerB->LogEvent("BasicFuncTests.stress_test_B"); + LogManagerB::UploadNow(); + + EXPECT_EQ(LogManagerB::FlushAndTeardown(), STATUS_SUCCESS); + EXPECT_EQ(PAL::PALTest::GetPalRefCount(), 1); + + EXPECT_EQ(LogManagerA::FlushAndTeardown(), STATUS_SUCCESS); + EXPECT_EQ(PAL::PALTest::GetPalRefCount(), 0); + } + + testRunning = false; + try + { + t.join(); + } + catch (std::exception) + { + // catch exception if can't join because the thread is already gone + }; + listener.dump(); + // Remove event listeners + for (auto evt : eventsList) + { + LogManagerB::RemoveEventListener(evt, listener); + LogManagerA::RemoveEventListener(evt, listener); + } +} #endif -#if 0 // XXX: [MG] - This test was never supposed to work! Because the URL is invalid, we won't get anything in receivedRequests +#if 0 // XXX: [MG] - This test was never supposed to work! Because the URL is invalid, we won't get anything in receivedRequests TEST_F(BasicFuncTests, networkProblemsDoNotDropEvents) { @@ -1316,7 +1478,7 @@ TEST_F(BasicFuncTests, networkProblemsDoNotDropEvents) } #endif -#if 0 // TODO: [MG] - re-enable this long-haul test +#if 0 // TODO: [MG] - re-enable this long-haul test TEST_F(BasicFuncTests, serverProblemsDropEventsAfterMaxRetryCount) { CleanStorage(); @@ -1368,4 +1530,4 @@ TEST_F(BasicFuncTests, serverProblemsDropEventsAfterMaxRetryCount) } } #endif -#endif // HAVE_MAT_DEFAULT_HTTP_CLIENT +#endif // HAVE_MAT_DEFAULT_HTTP_CLIENT From 32b9e0868e05b3f6641e57405faa55c802deb2b5 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 5 Mar 2020 01:52:03 -0800 Subject: [PATCH 03/11] Addressing code review comments from Jason: - provide a way to avoid infinite wait - extended the mutex to cover all possible request creation scenarios - idle-wait for task cancellation in smaller increments --- lib/http/HttpClient_WinInet.cpp | 235 +++++++------- lib/http/HttpClient_WinInet.hpp | 2 +- lib/include/public/ITaskDispatcher.hpp | 26 +- lib/include/public/LogManagerBase.hpp | 214 +++++++------ lib/include/public/Version.hpp | 6 +- lib/modules | 2 +- lib/pal/PAL.hpp | 41 ++- lib/pal/TaskDispatcher.hpp | 1 + lib/pal/WorkerThread.cpp | 30 +- lib/pal/universal/universal.vcxitems.filters | 2 +- lib/tpm/TransmissionPolicyManager.cpp | 49 ++- lib/tpm/TransmissionPolicyManager.hpp | 11 +- tests/functests/APITest.cpp | 24 +- tests/functests/BasicFuncTests.cpp | 312 +++++++++---------- tests/functests/BondDecoderTests.cpp | 2 +- tests/functests/FuncTests.vcxproj | 12 +- tests/functests/FuncTests.vcxproj.filters | 147 ++++----- tests/functests/LogSessionDataFuncTests.cpp | 4 +- 18 files changed, 582 insertions(+), 538 deletions(-) diff --git a/lib/http/HttpClient_WinInet.cpp b/lib/http/HttpClient_WinInet.cpp index 29da9409e..d5aba6311 100644 --- a/lib/http/HttpClient_WinInet.cpp +++ b/lib/http/HttpClient_WinInet.cpp @@ -12,7 +12,6 @@ #include #include -#include #include #include #include @@ -24,21 +23,18 @@ class WinInetRequestWrapper protected: HttpClient_WinInet& m_parent; std::string m_id; - IHttpResponseCallback* m_appCallback{ nullptr }; - HINTERNET m_hWinInetSession { nullptr }; - HINTERNET m_hWinInetRequest { nullptr }; + IHttpResponseCallback* m_appCallback {nullptr}; + HINTERNET m_hWinInetSession {nullptr}; + HINTERNET m_hWinInetRequest {nullptr}; SimpleHttpRequest* m_request; - BYTE m_buffer[1024] {}; - std::recursive_mutex m_connectingMutex; - bool isAborted { false }; - bool isCallbackCalled { false }; - + BYTE m_buffer[1024] {0}; + bool isCallbackCalled {false}; + bool isAborted {false}; public: WinInetRequestWrapper(HttpClient_WinInet& parent, SimpleHttpRequest* request) : m_parent(parent), - m_id(request->GetId()), - m_hWinInetRequest(nullptr), - m_request(request) + m_request(request), + m_id(request->GetId()) { LOG_TRACE("%p WinInetRequestWrapper()", this); } @@ -56,18 +52,32 @@ class WinInetRequestWrapper } } - /** - * Async cancel pending request - */ + /// + /// Asynchronously cancel pending request. This method is not directly calling + /// the object destructor, but rather hints the implementation to speed-up the + /// destruction. + /// + /// Two possible outcomes:. + //// + /// - set isAborted to true: cancel request without sending to WinInet stack, + /// in case if request has not been sent to WinInet stack yet. + //// + /// - close m_hWinInetRequest handle: WinInet fails all subsequent attempts to + /// use invalidated handle and aborts all pending WinInet worker threads on it. + /// In that case we complete with ERROR_INTERNET_OPERATION_CANCELLED. + /// + /// It may happen that we get some feedback from WinInet, i.e. we are canceling + /// at that same moment when the request is complete. In that case we process + /// completion in the callback (INTERNET_STATUS_REQUEST_COMPLETE). + /// void cancel() { - LOCKGUARD(m_connectingMutex); - // Cover the case where m_hWinInetRequest handle hasn't been created yet: + LOCKGUARD(m_parent.m_requestsMutex); isAborted = true; - // Cover the case where m_hWinInetRequest handle is created and has a callback registered: if (m_hWinInetRequest != nullptr) { ::InternetCloseHandle(m_hWinInetRequest); + // async request callback destroys the object } } @@ -119,112 +129,111 @@ class WinInetRequestWrapper return true; } + // Asynchronously send HTTP request and invoke response callback. + // Onwership semantics: send(...) method self-destroys *this* upon + // receiving WinInet callback. There must be absolutely no methods + // that attempt to use the object after triggering send on it. + // Send operation on request may be issued no more than once. + // + // Implementation details: + // + // lockguard around m_requestsMutex covers the following stages: + // - request added to map + // - URL parsed + // - DNS lookup performed, socket opened, SSL handshake + // - MS-Root SSL cert validation (if requested) + // - populating HTTP request headers + // - scheduling async(!) upload of HTTP post body + // + // Note that if any of the stages above fails, we invoke onRequestComplete(...). + // That method destroys "this" request object and in order to avoid + // any corruption we immediately return after invoking onRequestComplete(...). + // void send(IHttpResponseCallback* callback) { + LOCKGUARD(m_parent.m_requestsMutex); // Register app callback and request in HttpClient map m_appCallback = callback; + m_parent.m_requests[m_id] = this; + // If outside code asked us to abort that request before we could proceed with + // creating a WinInet handle, then clean it right away before proceeding with + // any async WinInet API calls. + if (isAborted) { - std::lock_guard lock(m_parent.m_requestsMutex); - m_parent.m_requests[m_id] = this; + // Request force-aborted before creating a WinInet handle. + DispatchEvent(OnConnectFailed); + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + return; } - { // Section of code that uses m_connectingMutex lock without a LOCKGUARD macro: - // - don't forget to unlock the mutex before calling "onRequestComplete" which destroys this object - // - don't access any object members after "onRequestComplete" (object is dead at this point) - m_connectingMutex.lock(); - - // If outside code asked us to abort that request before we could proceed with creating a WinInet - // handle on it, then clean it right away before proceeding with any async WinInet API calls. - if (isAborted) - { - // Request force-aborted before creating a WinInet handle. - DispatchEvent(OnConnectFailed); - m_connectingMutex.unlock(); - onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); - return; - } + DispatchEvent(OnConnecting); + URL_COMPONENTSA urlc; + memset(&urlc, 0, sizeof(urlc)); + urlc.dwStructSize = sizeof(urlc); + char hostname[256] = { 0 }; + urlc.lpszHostName = hostname; + urlc.dwHostNameLength = sizeof(hostname); + char path[1024] = { 0 }; + urlc.lpszUrlPath = path; + urlc.dwUrlPathLength = sizeof(path); + if (!::InternetCrackUrlA(m_request->m_url.data(), (DWORD)m_request->m_url.size(), 0, &urlc)) + { + DWORD dwError = ::GetLastError(); + LOG_WARN("InternetCrackUrl() failed: dwError=%d url=%s", dwError, m_request->m_url.data()); + // Invalid URL passed to WinInet API + DispatchEvent(OnConnectFailed); + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + return; + } - DispatchEvent(OnConnecting); - - URL_COMPONENTSA urlc; - memset(&urlc, 0, sizeof(urlc)); - urlc.dwStructSize = sizeof(urlc); - char hostname[256]; - urlc.lpszHostName = hostname; - urlc.dwHostNameLength = sizeof(hostname); - char path[1024]; - urlc.lpszUrlPath = path; - urlc.dwUrlPathLength = sizeof(path); - if (!::InternetCrackUrlA(m_request->m_url.data(), (DWORD)m_request->m_url.size(), 0, &urlc)) - { - DWORD dwError = ::GetLastError(); - LOG_WARN("InternetCrackUrl() failed: dwError=%d url=%s", dwError, m_request->m_url.data()); - // Invalid URL passed to WinInet API - DispatchEvent(OnConnectFailed); - m_connectingMutex.unlock(); - onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); - return; - } + m_hWinInetSession = ::InternetConnectA(m_parent.m_hInternet, hostname, urlc.nPort, + NULL, NULL, INTERNET_SERVICE_HTTP, 0, reinterpret_cast(this)); + if (m_hWinInetSession == NULL) { + DWORD dwError = ::GetLastError(); + LOG_WARN("InternetConnect() failed: %d", dwError); + // Cannot connect to host + DispatchEvent(OnConnectFailed); + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + return; + } + // TODO: Session handle for the same target should be cached across requests to enable keep-alive. + + PCSTR szAcceptTypes[] = {"*/*", NULL}; + m_hWinInetRequest = ::HttpOpenRequestA( + m_hWinInetSession, m_request->m_method.c_str(), path, NULL, NULL, szAcceptTypes, + INTERNET_FLAG_KEEP_CONNECTION | INTERNET_FLAG_NO_AUTH | INTERNET_FLAG_NO_CACHE_WRITE | + INTERNET_FLAG_NO_COOKIES | INTERNET_FLAG_NO_UI | INTERNET_FLAG_PRAGMA_NOCACHE | + INTERNET_FLAG_RELOAD | (urlc.nScheme == INTERNET_SCHEME_HTTPS ? INTERNET_FLAG_SECURE : 0), + reinterpret_cast(this)); + if (m_hWinInetRequest == NULL) { + DWORD dwError = ::GetLastError(); + LOG_WARN("HttpOpenRequest() failed: %d", dwError); + // Request cannot be opened to given URL because of some connectivity issue + DispatchEvent(OnConnectFailed); + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + return; + } - m_hWinInetSession = ::InternetConnectA(m_parent.m_hInternet, hostname, urlc.nPort, - NULL, NULL, INTERNET_SERVICE_HTTP, 0, reinterpret_cast(this)); - if (m_hWinInetSession == NULL) - { - DWORD dwError = ::GetLastError(); - LOG_WARN("InternetConnect() failed: %d", dwError); - // Cannot connect to host - DispatchEvent(OnConnectFailed); - m_connectingMutex.unlock(); - onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); - return; - } - // TODO: Session handle for the same target should be cached across requests to enable keep-alive. - - PCSTR szAcceptTypes[] = {"*/*", NULL}; - m_hWinInetRequest = ::HttpOpenRequestA( - m_hWinInetSession, m_request->m_method.c_str(), path, NULL, NULL, szAcceptTypes, - INTERNET_FLAG_KEEP_CONNECTION | INTERNET_FLAG_NO_AUTH | INTERNET_FLAG_NO_CACHE_WRITE | - INTERNET_FLAG_NO_COOKIES | INTERNET_FLAG_NO_UI | INTERNET_FLAG_PRAGMA_NOCACHE | - INTERNET_FLAG_RELOAD | (urlc.nScheme == INTERNET_SCHEME_HTTPS ? INTERNET_FLAG_SECURE : 0), - reinterpret_cast(this)); - if (m_hWinInetRequest == NULL) + /* Perform optional MS Root certificate check for certain end-point URLs */ + if (m_parent.IsMsRootCheckRequired()) + { + if (!isMsRootCert()) { - DWORD dwError = ::GetLastError(); - LOG_WARN("HttpOpenRequest() failed: %d", dwError); - // Request cannot be opened to given URL because of some connectivity issue + // Request cannot be completed: end-point certificate is not MS-Rooted DispatchEvent(OnConnectFailed); - m_connectingMutex.unlock(); - onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + onRequestComplete(ERROR_INTERNET_SEC_INVALID_CERT); return; } - - /* Perform optional MS Root certificate check for certain end-point URLs */ - if (m_parent.IsMsRootCheckRequired()) - { - if (!isMsRootCert()) - { - // Request cannot be completed: end-point certificate is not MS-Rooted. - DispatchEvent(OnConnectFailed); - m_connectingMutex.unlock(); - onRequestComplete(ERROR_INTERNET_SEC_INVALID_CERT); - return; - } - } - - ::InternetSetStatusCallback(m_hWinInetRequest, &WinInetRequestWrapper::winInetCallback); - // At this point this->cancel() is able to async-destroy the request object by calling - // ::InternetCloseHandle(...) : winInetCallback is invoked in context of WinInet thread - // pool worker thread, which subsequently calls onRequestComplete(...) to destroy. - m_connectingMutex.unlock(); } - // End of section of code that uses m_connectingMutex lock + + ::InternetSetStatusCallback(m_hWinInetRequest, &WinInetRequestWrapper::winInetCallback); std::ostringstream os; - for (auto const& header : m_request->m_headers) - { + for (auto const& header : m_request->m_headers) { os << header.first << ": " << header.second << "\r\n"; } + if (!::HttpAddRequestHeadersA(m_hWinInetRequest, os.str().data(), static_cast(os.tellp()), HTTP_ADDREQ_FLAG_ADD | HTTP_ADDREQ_FLAG_REPLACE)) { DWORD dwError = ::GetLastError(); @@ -273,7 +282,7 @@ class WinInetRequestWrapper } case INTERNET_STATUS_HANDLE_CLOSING: - // HANDLE_CLOSING should always come after REQUEST_COMPLETE/. When (and if) + // HANDLE_CLOSING should always come after REQUEST_COMPLETE. When (and if) // it (ever) happens, WinInetRequestWrapper* self pointer may point to object // that has been already destroyed. We do not perform any actions on it. return; @@ -392,6 +401,7 @@ class WinInetRequestWrapper // We may still invoke OnHttpResponse(...) below for this positive as well as other negative // cases where there was a short-read, connection failuire or timeout on reading the response. DispatchEvent(OnResponse); + } else { switch (dwError) { case ERROR_INTERNET_OPERATION_CANCELLED: @@ -431,7 +441,7 @@ class WinInetRequestWrapper } } - assert(isCallbackCalled==false); + assert(isCallbackCalled == false); if (!isCallbackCalled) { // Only one WinInet worker thread may invoke async callback for a given request at any given moment of time. @@ -462,9 +472,13 @@ HttpClient_WinInet::~HttpClient_WinInet() ::InternetCloseHandle(m_hInternet); } +/** + * This method is called exclusively from onRequestComplete . + * No other code paths that lead to request destruction. + */ void HttpClient_WinInet::erase(std::string const& id) { - std::lock_guard lock(m_requestsMutex); + LOCKGUARD(m_requestsMutex); auto it = m_requests.find(id); if (it != m_requests.end()) { auto req = it->second; @@ -489,11 +503,10 @@ void HttpClient_WinInet::SendRequestAsync(IHttpRequest* request, IHttpResponseCa void HttpClient_WinInet::CancelRequestAsync(std::string const& id) { - WinInetRequestWrapper* request = nullptr; LOCKGUARD(m_requestsMutex); auto it = m_requests.find(id); if (it != m_requests.end()) { - request = it->second; + auto request = it->second; if (request) { request->cancel(); } @@ -506,7 +519,7 @@ void HttpClient_WinInet::CancelAllRequests() // vector of all request IDs std::vector ids; { - std::lock_guard lock(m_requestsMutex); + LOCKGUARD(m_requestsMutex); for (auto const& item : m_requests) { ids.push_back(item.first); } diff --git a/lib/http/HttpClient_WinInet.hpp b/lib/http/HttpClient_WinInet.hpp index d719d6c91..a95143127 100644 --- a/lib/http/HttpClient_WinInet.hpp +++ b/lib/http/HttpClient_WinInet.hpp @@ -36,7 +36,7 @@ class HttpClient_WinInet : public IHttpClient { protected: HINTERNET m_hInternet; - std::mutex m_requestsMutex; + std::recursive_mutex m_requestsMutex; std::map m_requests; static unsigned s_nextRequestId; bool m_msRootCheck; diff --git a/lib/include/public/ITaskDispatcher.hpp b/lib/include/public/ITaskDispatcher.hpp index ba0360b90..ffc6c36a8 100644 --- a/lib/include/public/ITaskDispatcher.hpp +++ b/lib/include/public/ITaskDispatcher.hpp @@ -1,11 +1,12 @@ // Copyright (c) Microsoft. All rights reserved. -#ifndef ITASKDISPATHER_HPP -#define ITASKDISPATHER_HPP +#ifndef ITASKDISPATCHER_HPP +#define ITASKDISPATCHER_HPP #include "IModule.hpp" #include "Version.hpp" #include "ctmacros.hpp" +#include #include #include @@ -19,6 +20,15 @@ namespace ARIASDK_NS_BEGIN /// class Task { + /// + /// Atomic counter that returns sequentially incrementing unique Task ID + /// + static uint64_t GetNewTid() + { + std::atomic lastTid; + return lastTid.fetch_add(1); + } + public: /// /// Type of work item @@ -47,11 +57,21 @@ namespace ARIASDK_NS_BEGIN Cancelled } Type; + Task() : + tid(GetNewTid()) + {}; + /// /// The time (in milliseconds since epoch) when this work item should be executed /// uint64_t TargetTime; + /// + /// Unique Task Id. + /// TODO: [maxgolov] - use this Task Id for task cancellation instead of raw ptr + /// + uint64_t tid; + /// /// The Task class destructor. /// @@ -105,4 +125,4 @@ namespace ARIASDK_NS_BEGIN } ARIASDK_NS_END -#endif // ITASKDISPATHER_HPP +#endif // ITASKDISPATCHER_HPP diff --git a/lib/include/public/LogManagerBase.hpp b/lib/include/public/LogManagerBase.hpp index 70965b904..67f9110b2 100644 --- a/lib/include/public/LogManagerBase.hpp +++ b/lib/include/public/LogManagerBase.hpp @@ -2,8 +2,8 @@ #ifndef MAT_LOGMANAGER_HPP #define MAT_LOGMANAGER_HPP -#include "CommonFields.h" #include "ctmacros.hpp" +#include "CommonFields.h" #if (HAVE_EXCEPTIONS) #include @@ -11,7 +11,7 @@ #ifdef _MSC_VER #pragma warning(push) -#pragma warning(disable : 4459 4100 4121 4068) +#pragma warning(disable:4459 4100 4121 4068) #endif #pragma clang diagnostic push @@ -19,10 +19,8 @@ #ifdef _MANAGED #include -public -ref class LogManagerLock -{ - public: +public ref class LogManagerLock { +public: static Object ^ lock = gcnew Object(); }; #else @@ -31,71 +29,71 @@ ref class LogManagerLock #include "LogManagerProvider.hpp" -#define LM_SAFE_CALL(method, ...) \ - { \ - LM_LOCKGUARD(stateLock()); \ - if (nullptr != instance) \ - { \ - instance->method(__VA_ARGS__); \ - return STATUS_SUCCESS; \ - } \ - return STATUS_EFAIL; \ +#define LM_SAFE_CALL(method , ... ) \ + { \ + LM_LOCKGUARD(stateLock()); \ + if (nullptr != instance) \ + { \ + instance-> method ( __VA_ARGS__); \ + return STATUS_SUCCESS; \ + } \ + return STATUS_EFAIL; \ } -#define LM_SAFE_CALL_RETURN(method, ...) \ - { \ - LM_LOCKGUARD(stateLock()); \ - if (nullptr != instance) \ - { \ - return instance->method(__VA_ARGS__); \ - } \ +#define LM_SAFE_CALL_RETURN(method , ... ) \ + { \ + LM_LOCKGUARD(stateLock()); \ + if (nullptr != instance) \ + { \ + return instance-> method ( __VA_ARGS__);\ + } \ } -#define LM_SAFE_CALL_STR(method, ...) \ - { \ - LM_LOCKGUARD(stateLock()); \ - if (nullptr != instance) \ - { \ - return instance->method(__VA_ARGS__); \ - } \ - return ""; \ +#define LM_SAFE_CALL_STR(method , ... ) \ + { \ + LM_LOCKGUARD(stateLock()); \ + if (nullptr != instance) \ + { \ + return instance-> method ( __VA_ARGS__);\ + } \ + return ""; \ } -#define LM_SAFE_CALL_PTR(method, ...) \ - { \ - LM_LOCKGUARD(stateLock()); \ - if (nullptr != instance) \ - { \ - return instance->method(__VA_ARGS__); \ - } \ - return nullptr; \ +#define LM_SAFE_CALL_PTR(method , ... ) \ + { \ + LM_LOCKGUARD(stateLock()); \ + if (nullptr != instance) \ + { \ + return instance-> method ( __VA_ARGS__);\ + } \ + return nullptr; \ } -#define LM_SAFE_CALL_PTRREF(method, ...) \ - { \ - LM_LOCKGUARD(stateLock()); \ - if (nullptr != instance) \ - { \ - return &(instance->method(__VA_ARGS__)); \ - } \ - return nullptr; \ +#define LM_SAFE_CALL_PTRREF(method , ... ) \ + { \ + LM_LOCKGUARD(stateLock()); \ + if (nullptr != instance) \ + { \ + return &(instance-> method ( __VA_ARGS__));\ + } \ + return nullptr; \ } -#define LM_SAFE_CALL_VOID(method, ...) \ - { \ - LM_LOCKGUARD(stateLock()); \ - if (nullptr != instance) \ - { \ - instance->method(__VA_ARGS__); \ - return; \ - } \ +#define LM_SAFE_CALL_VOID(method , ... ) \ + { \ + LM_LOCKGUARD(stateLock()); \ + if (nullptr != instance) \ + { \ + instance-> method ( __VA_ARGS__); \ + return; \ + } \ } #ifdef _MANAGED #define LM_LOCKGUARD(macro_mutex) msclr::lock l(LogManagerLock::lock); #else -#define LM_LOCKGUARD(macro_mutex) \ - std::lock_guard TOKENPASTE2(__guard_, __LINE__)(macro_mutex); +#define LM_LOCKGUARD(macro_mutex) \ + std::lock_guard TOKENPASTE2(__guard_, __LINE__) (macro_mutex); #endif namespace ARIASDK_NS_BEGIN @@ -103,11 +101,9 @@ namespace ARIASDK_NS_BEGIN #if (HAVE_EXCEPTIONS) class LogManagerNotInitializedException : public std::runtime_error { - public: - LogManagerNotInitializedException(const char* message) noexcept : - std::runtime_error(message) - { - } + public: + LogManagerNotInitializedException(const char* message) noexcept + : std::runtime_error(message) { } }; #endif @@ -116,39 +112,39 @@ namespace ARIASDK_NS_BEGIN /// is running in "host" mode and all LogController methods should be accessible to the /// caller. /// - static constexpr const char* HOST_MODE = "hostMode"; + static constexpr const char * HOST_MODE = "hostMode"; /// /// This class is used to manage the Events logging system /// - template - class LogManagerBase + template class LogManagerBase { static_assert(std::is_base_of::value, "ModuleConfiguration must derive from LogConfiguration"); - public: + public: using basetype = LogManagerBase; - protected: + protected: + /// /// LogManagerBase constructor /// - LogManagerBase(){}; + LogManagerBase() {}; /// /// LogManager copy constructor /// - LogManagerBase(const LogManagerBase&){}; + LogManagerBase(const LogManagerBase&) {}; /// /// [not implemented] LogManager assignment operator /// - LogManagerBase& operator=(const LogManagerBase&){}; + LogManagerBase& operator=(const LogManagerBase&) {}; /// /// LogManager destructor /// - virtual ~LogManagerBase(){}; + virtual ~LogManagerBase() {}; #ifndef _MANAGED /// @@ -171,7 +167,7 @@ namespace ARIASDK_NS_BEGIN /// /// Concrete instance for servicing all singleton calls /// - static ILogManager* instance; + static ILogManager* instance; /// /// Debug event source associated with this singleton @@ -185,10 +181,11 @@ namespace ARIASDK_NS_BEGIN static const char* GetPrimaryToken() { ILogConfiguration& config = GetLogConfiguration(); - return (const char*)(config[CFG_STR_PRIMARY_TOKEN]); + return (const char *)(config[CFG_STR_PRIMARY_TOKEN]); } - public: + public: + /// /// Returns this module LogConfiguration /// @@ -204,7 +201,7 @@ namespace ARIASDK_NS_BEGIN /// A logger instance instantiated with the default tenantToken. static ILogger* Initialize() { - return Initialize(std::string{}); + return Initialize(std::string {}); } /// @@ -214,7 +211,7 @@ namespace ARIASDK_NS_BEGIN /// A logger instance instantiated with the tenantToken. inline static ILogger* Initialize(const std::string& tenantToken) { - return Initialize(tenantToken, GetLogConfiguration()); + return Initialize(tenantToken, GetLogConfiguration()); } /// @@ -225,7 +222,8 @@ namespace ARIASDK_NS_BEGIN /// A logger instance instantiated with the tenantToken. static ILogger* Initialize( const std::string& tenantToken, - ILogConfiguration& configuration) + ILogConfiguration& configuration + ) { LM_LOCKGUARD(stateLock()); ILogConfiguration& currentConfig = GetLogConfiguration(); @@ -234,12 +232,12 @@ namespace ARIASDK_NS_BEGIN // Copy alternate configuration into currentConfig if (&configuration != ¤tConfig) { - for (const auto& kv : *configuration) + for (const auto &kv : *configuration) { currentConfig[kv.first.c_str()] = kv.second; } - for (const auto& kv : configuration.GetModules()) + for (const auto &kv : configuration.GetModules()) { currentConfig.AddModule(kv.first.c_str(), kv.second); } @@ -256,8 +254,7 @@ namespace ARIASDK_NS_BEGIN instance->AttachEventSource(GetDebugEventSource()); return instance->GetLogger(currentConfig[CFG_STR_PRIMARY_TOKEN]); } - else - { + else { // TODO: [MG] - decide what to do if someone's doing re-Initialize. // For now Re-initialize works as GetLogger with an alternate token. // We may decide to assert(tenantToken==primaryToken) ... @@ -275,7 +272,7 @@ namespace ARIASDK_NS_BEGIN static status_t FlushAndTeardown() { LM_LOCKGUARD(stateLock()); -#ifdef NO_TEARDOWN // Avoid destroying our ILogManager instance on teardown +#ifdef NO_TEARDOWN // Avoid destroying our ILogManager instance on teardown LM_SAFE_CALL(Flush); LM_SAFE_CALL(UploadNow); return STATUS_SUCCESS; @@ -315,12 +312,12 @@ namespace ARIASDK_NS_BEGIN LM_LOCKGUARD(stateLock()); if (isHost()) LM_SAFE_CALL(GetLogController()->UploadNow); - return STATUS_EPERM; // Permission denied + return STATUS_EPERM; // Permission denied } /// /// Flush any pending telemetry events in memory to disk to reduce possible data loss as seen necessary. - /// This function can be very expensive so should be used sparingly. OS will block the calling thread + /// This function can be very expensive so should be used sparingly. OS will block the calling thread /// and might flush the global file buffers, i.e. all buffered filesystem data, to disk, which could be /// time consuming. /// @@ -328,7 +325,7 @@ namespace ARIASDK_NS_BEGIN { if (isHost()) LM_SAFE_CALL(GetLogController()->Flush); - return STATUS_EPERM; // Permission denied + return STATUS_EPERM; // Permission denied } /// @@ -339,7 +336,7 @@ namespace ARIASDK_NS_BEGIN { if (isHost()) LM_SAFE_CALL(GetLogController()->PauseTransmission); - return STATUS_EPERM; // Permission denied + return STATUS_EPERM; // Permission denied } /// @@ -349,13 +346,13 @@ namespace ARIASDK_NS_BEGIN { if (isHost()) LM_SAFE_CALL(GetLogController()->ResumeTransmission); - return STATUS_EPERM; // Permission denied + return STATUS_EPERM; // Permission denied } /// /// Sets transmit profile for event transmission to one of the built-in profiles. /// A transmit profile is a collection of hardware and system settings (like network connectivity, power state) - /// based on which to determine how events are to be transmitted. + /// based on which to determine how events are to be transmitted. /// /// Transmit profile /// This function doesn't return any value because it always succeeds. @@ -363,13 +360,13 @@ namespace ARIASDK_NS_BEGIN { if (isHost()) LM_SAFE_CALL(GetLogController()->SetTransmitProfile, profile); - return STATUS_EPERM; // Permission denied + return STATUS_EPERM; // Permission denied } /// /// Sets transmit profile for event transmission. /// A transmit profile is a collection of hardware and system settings (like network connectivity, power state) - /// based on which to determine how events are to be transmitted. + /// based on which to determine how events are to be transmitted. /// /// Transmit profile /// true if profile is successfully applied, false otherwise @@ -377,7 +374,7 @@ namespace ARIASDK_NS_BEGIN { if (isHost()) LM_SAFE_CALL(GetLogController()->SetTransmitProfile, profile); - return STATUS_EPERM; // Permission denied + return STATUS_EPERM; // Permission denied } /// @@ -389,7 +386,7 @@ namespace ARIASDK_NS_BEGIN { if (isHost()) LM_SAFE_CALL(GetLogController()->LoadTransmitProfiles, profiles_json); - return STATUS_EPERM; // Permission denied + return STATUS_EPERM; // Permission denied } /// @@ -399,7 +396,7 @@ namespace ARIASDK_NS_BEGIN { if (isHost()) LM_SAFE_CALL(GetLogController()->ResetTransmitProfiles); - return STATUS_EPERM; // Permission denied + return STATUS_EPERM; // Permission denied } #endif @@ -410,9 +407,9 @@ namespace ARIASDK_NS_BEGIN LM_SAFE_CALL_STR(GetTransmitProfileName); /// - /// Retrieve an ISemanticContext interface through which to specify context information + /// Retrieve an ISemanticContext interface through which to specify context information /// such as device, system, hardware and user information. - /// Context information set via this API will apply to all logger instance unless they + /// Context information set via this API will apply to all logger instance unless they /// are overwritten by individual logger instance. /// /// ISemanticContext interface pointer @@ -421,7 +418,7 @@ namespace ARIASDK_NS_BEGIN /// /// Adds or overrides a property of the custom context for the telemetry logging system. - /// Context information set here applies to events generated by all ILogger instances + /// Context information set here applies to events generated by all ILogger instances /// unless it is overwritten on a particular ILogger instance. /// /// Name of the context property @@ -432,13 +429,13 @@ namespace ARIASDK_NS_BEGIN /// /// Adds or overrides a property of the custom context for the telemetry logging system. - /// Context information set here applies to events generated by all ILogger instances + /// Context information set here applies to events generated by all ILogger instances /// unless it is overwritten on a particular ILogger instance. /// /// Name of the context property /// Value of the context property /// PIIKind of the context with PiiKind_None as the default - static status_t SetContext(const std::string& name, const char* value, PiiKind piiKind = PiiKind_None) + static status_t SetContext(const std::string& name, const char *value, PiiKind piiKind = PiiKind_None) LM_SAFE_CALL(SetContext, name, std::string(value), piiKind); /// @@ -463,7 +460,7 @@ namespace ARIASDK_NS_BEGIN /// /// Name of the property /// 8-bit Integer value of the property - static status_t SetContext(const std::string& name, int8_t value, PiiKind piiKind = PiiKind_None) + static status_t SetContext(const std::string& name, int8_t value, PiiKind piiKind = PiiKind_None) LM_SAFE_CALL(SetContext, name, (int64_t)value, piiKind); /// @@ -490,7 +487,7 @@ namespace ARIASDK_NS_BEGIN /// /// Name of the property /// 8-bit unsigned integer value of the property - static status_t SetContext(const std::string& name, uint8_t value, PiiKind piiKind = PiiKind_None) + static status_t SetContext(const std::string& name, uint8_t value, PiiKind piiKind = PiiKind_None) LM_SAFE_CALL(SetContext, name, (int64_t)value, piiKind); /// @@ -596,7 +593,7 @@ namespace ARIASDK_NS_BEGIN /// /// Add Debug callback /// - static void AddEventListener(DebugEventType type, DebugEventListener& listener) + static void AddEventListener(DebugEventType type, DebugEventListener &listener) { GetDebugEventSource().AddEventListener(type, listener); } @@ -604,7 +601,7 @@ namespace ARIASDK_NS_BEGIN /// /// Remove Debug callback /// - static void RemoveEventListener(DebugEventType type, DebugEventListener& listener) + static void RemoveEventListener(DebugEventType type, DebugEventListener &listener) { GetDebugEventSource().RemoveEventListener(type, listener); } @@ -691,17 +688,18 @@ namespace ARIASDK_NS_BEGIN // that causes improper global static templated member initialization: // https://developercommunity.visualstudio.com/content/problem/134886/initialization-of-template-static-variable-wrong.html // -#define DEFINE_LOGMANAGER(LogManagerClass, LogConfigurationClass) \ - ILogManager* LogManagerClass::instance = nullptr; +#define DEFINE_LOGMANAGER(LogManagerClass, LogConfigurationClass) \ + ILogManager* LogManagerClass::instance = nullptr; +#elif defined(__APPLE__) && defined(MAT_USE_WEAK_LOGMANAGER) +#define DEFINE_LOGMANAGER(LogManagerClass, LogConfigurationClass) \ + template<> __attribute__((weak)) ILogManager* LogManagerBase::instance {}; #else // ISO C++ -compliant declaration -#define DEFINE_LOGMANAGER(LogManagerClass, LogConfigurationClass) \ - template <> \ - ILogManager* LogManagerBase::instance{}; +#define DEFINE_LOGMANAGER(LogManagerClass, LogConfigurationClass) \ + template<> ILogManager* LogManagerBase::instance {}; #endif -} -ARIASDK_NS_END +} ARIASDK_NS_END #pragma clang diagnostic pop diff --git a/lib/include/public/Version.hpp b/lib/include/public/Version.hpp index 8ab513207..bd6fb6092 100644 --- a/lib/include/public/Version.hpp +++ b/lib/include/public/Version.hpp @@ -3,8 +3,8 @@ #define MAT_VERSION_HPP // WARNING: DO NOT MODIFY THIS FILE! // This file has been automatically generated, manual changes will be lost. -#define BUILD_VERSION_STR "3.3.29.1" -#define BUILD_VERSION 3,3,29,1 +#define BUILD_VERSION_STR "3.3.64.1" +#define BUILD_VERSION 3,3,64,1 #ifndef RESOURCE_COMPILER_INVOKED #include @@ -30,7 +30,7 @@ namespace ARIASDK_NS_BEGIN { uint64_t const Version = ((uint64_t)3 << 48) | ((uint64_t)3 << 32) | - ((uint64_t)29 << 16) | + ((uint64_t)64 << 16) | ((uint64_t)1); } ARIASDK_NS_END diff --git a/lib/modules b/lib/modules index 6ba663a91..9e82bf4fb 160000 --- a/lib/modules +++ b/lib/modules @@ -1 +1 @@ -Subproject commit 6ba663a91236d8ca8c7adafb5a4f71a7f47a7e82 +Subproject commit 9e82bf4fb4a863c96b6ec0c6ec8fb1314b25daef diff --git a/lib/pal/PAL.hpp b/lib/pal/PAL.hpp index fbdba3021..9a28f88d7 100644 --- a/lib/pal/PAL.hpp +++ b/lib/pal/PAL.hpp @@ -2,9 +2,9 @@ #ifndef PAL_HPP #define PAL_HPP -#include "DebugTrace.hpp" -#include "Version.hpp" #include "mat/config.h" +#include "Version.hpp" +#include "DebugTrace.hpp" #ifdef HAVE_MAT_EXP #define ECS_SUPP "ECS" @@ -12,9 +12,9 @@ #define ECS_SUPP "No" #endif -#include "DeviceInformationImpl.hpp" -#include "NetworkInformationImpl.hpp" #include "SystemInformationImpl.hpp" +#include "NetworkInformationImpl.hpp" +#include "DeviceInformationImpl.hpp" #include "ISemanticContext.hpp" @@ -26,8 +26,8 @@ #ifndef WIN32_LEAN_AND_MEAN #define WIN32_LEAN_AND_MEAN #endif -#include #include +#include #else /* POSIX */ #define PATH_SEPARATOR_CHAR '/' @@ -39,28 +39,27 @@ #include #include -#include -#include -#include -#include #include #include #include -#include #include #include -#include #include +#include +#include +#include +#include +#include +#include -#include "WorkerThread.hpp" #include "api/IRuntimeConfig.hpp" #include "typename.hpp" +#include "WorkerThread.hpp" namespace ARIASDK_NS_BEGIN { void print_backtrace(); -} -ARIASDK_NS_END +} ARIASDK_NS_END namespace PAL_NS_BEGIN { @@ -71,9 +70,8 @@ namespace PAL_NS_BEGIN class PlatformAbstractionLayer { - friend class PALTest; - - public: + friend class PALTest; + public: void sleep(unsigned delayMs) const noexcept; const std::string& getSdkVersion() const; @@ -97,7 +95,7 @@ namespace PAL_NS_BEGIN void initialize(IRuntimeConfig& configuration); void shutdown(); - + INetworkInformation* GetNetworkInformation() const noexcept; IDeviceInformation* GetDeviceInformation() const noexcept; ISystemInformation* GetSystemInformation() const noexcept; @@ -108,8 +106,8 @@ namespace PAL_NS_BEGIN MATSDK_LOG_DECL_COMPONENT_CLASS(); - private: - volatile std::atomic m_palStarted{0}; + private: + volatile std::atomic m_palStarted { 0 }; std::shared_ptr m_taskDispatcher; ISystemInformation* m_SystemInformation; INetworkInformation* m_NetworkInformation; @@ -237,7 +235,6 @@ namespace PAL_NS_BEGIN return GetPAL().RegisterIkeyWithWindowsTelemetry(ikeyin, storageSize, uploadQuotaSize); } -} -PAL_NS_END +} PAL_NS_END #endif diff --git a/lib/pal/TaskDispatcher.hpp b/lib/pal/TaskDispatcher.hpp index e564f9dd6..391ac11ca 100644 --- a/lib/pal/TaskDispatcher.hpp +++ b/lib/pal/TaskDispatcher.hpp @@ -1,3 +1,4 @@ +// clang-format off #ifndef TASK_DISPATCHER_HPP #define TASK_DISPATCHER_HPP diff --git a/lib/pal/WorkerThread.cpp b/lib/pal/WorkerThread.cpp index 1a59177a4..7ca1320b7 100644 --- a/lib/pal/WorkerThread.cpp +++ b/lib/pal/WorkerThread.cpp @@ -1,3 +1,4 @@ +// clang-format off #include "pal/WorkerThread.hpp" #include "pal/PAL.hpp" @@ -6,15 +7,18 @@ /* Maximum scheduler interval for SDK is 1 hour required for clamping in case of monotonic clock drift */ #define MAX_FUTURE_DELTA_MS (60 * 60 * 1000) -/* Polling interval for task cancellation */ -#define TASK_CANCEL_WAIT_MS 100 +// Polling interval for task cancellation can be customized at compile-time +#ifndef TASK_CANCEL_WAIT_MS +#define TASK_CANCEL_WAIT_MS 50 +#endif namespace PAL_NS_BEGIN { class WorkerThreadShutdownItem : public Task { public: - WorkerThreadShutdownItem() + WorkerThreadShutdownItem() : + Task() { Type = MAT::Task::Shutdown; } @@ -91,6 +95,19 @@ namespace PAL_NS_BEGIN { m_event.post(); } + // TODO: [maxgolov] - method has to be refactored to allow cancellation by Task tid. + // Otherwise we may run into a situation where we wait on a 'wrong task' completion. + // In case if the task we were supposed to be waiting on is done running and destroyed, + // but while we idle-wait for TASK_CANCEL_WAIT_MS - a new task obj got allocated on + // heap with the same exact raw ptr. The chance of that collision is slim and the only + // side-effect is we'd idle-wait slightly longer until the new task is done. + // + // It would also make sense to return the following cancellation status: + // - task not found + // - task found and cancelled + // - task found, but not cancelled - ran to completion. + // - task found, but not cancelled - still running. + // bool Cancel(MAT::Task* item, uint64_t waitTime) override { if (item == nullptr) @@ -109,9 +126,12 @@ namespace PAL_NS_BEGIN { PAL::sleep(TASK_CANCEL_WAIT_MS); waitTime -= TASK_CANCEL_WAIT_MS; } - /* Either waited long enough or the task is still executing */ } - return (m_itemInProgress == item); + /* Either waited long enough or the task is still executing. Return: + - true - if new item in progress is another new item + - false - if the item in progress is still the item we were waiting on + */ + return (m_itemInProgress != item); } { diff --git a/lib/pal/universal/universal.vcxitems.filters b/lib/pal/universal/universal.vcxitems.filters index 166fe746a..7b4700534 100644 --- a/lib/pal/universal/universal.vcxitems.filters +++ b/lib/pal/universal/universal.vcxitems.filters @@ -6,7 +6,7 @@ - + diff --git a/lib/tpm/TransmissionPolicyManager.cpp b/lib/tpm/TransmissionPolicyManager.cpp index 66ff864cc..146be6489 100644 --- a/lib/tpm/TransmissionPolicyManager.cpp +++ b/lib/tpm/TransmissionPolicyManager.cpp @@ -6,16 +6,16 @@ #include -#define ABS64(a, b) ((a > b) ? (a - b) : (b - a)) +#define ABS64(a,b) ((a>b)?(a-b):(b-a)) -namespace ARIASDK_NS_BEGIN -{ - int const DEFAULT_DELAY_SEND_HTTP = 2 * 1000; // 2 sec +namespace ARIASDK_NS_BEGIN { + + int const DEFAULT_DELAY_SEND_HTTP = 2 * 1000; // 2 sec MATSDK_LOG_INST_COMPONENT_CLASS(TransmissionPolicyManager, "EventsSDK.TPM", "Events telemetry client - TransmissionPolicyManager class"); - TransmissionPolicyManager::TransmissionPolicyManager(ITelemetrySystem& system, ITaskDispatcher& taskDispatcher, IBandwidthController* bandwidthController) : - m_lock(), + TransmissionPolicyManager::TransmissionPolicyManager(ITelemetrySystem& system, ITaskDispatcher& taskDispatcher, IBandwidthController* bandwidthController) + : m_lock(), m_system(system), m_taskDispatcher(taskDispatcher), m_config(m_system.getConfig()), @@ -86,7 +86,7 @@ namespace ARIASDK_NS_BEGIN { return; } - if (uploadCount() >= static_cast(m_config[CFG_INT_MAX_PENDING_REQ])) + if (uploadCount() >= static_cast(m_config[CFG_INT_MAX_PENDING_REQ]) ) { LOG_TRACE("Maximum number of HTTP requests reached"); return; @@ -98,7 +98,7 @@ namespace ARIASDK_NS_BEGIN return; } - if ((!force) && (m_isUploadScheduled)) + if ((!force)&&(m_isUploadScheduled)) { if (m_runningLatency > latency) { @@ -139,7 +139,7 @@ namespace ARIASDK_NS_BEGIN void TransmissionPolicyManager::uploadAsync(EventLatency latency) { - m_isUploadScheduled = false; // Allow to schedule another uploadAsync + m_isUploadScheduled = false; // Allow to schedule another uploadAsync m_runningLatency = latency; m_scheduledUploadTime = std::numeric_limits::max(); @@ -153,22 +153,19 @@ namespace ARIASDK_NS_BEGIN } } -#ifdef ENABLE_BW_CONTROLLER /* Bandwidth controller is not currently supported */ - if (m_bandwidthController) - { +#ifdef ENABLE_BW_CONTROLLER /* Bandwidth controller is not currently supported */ + if (m_bandwidthController) { unsigned proposedBandwidthBps = m_bandwidthController->GetProposedBandwidthBps(); unsigned minimumBandwidthBps = m_config.GetMinimumUploadBandwidthBps(); - if (proposedBandwidthBps >= minimumBandwidthBps) - { + if (proposedBandwidthBps >= minimumBandwidthBps) { LOG_TRACE("Bandwidth controller proposed sufficient bandwidth %u bytes/sec (minimum accepted is %u)", - proposedBandwidthBps, minimumBandwidthBps); + proposedBandwidthBps, minimumBandwidthBps); } - else - { + else { unsigned delayMs = 1000; LOG_INFO("Bandwidth controller proposed bandwidth %u bytes/sec but minimum accepted is %u, will retry %u ms later", - proposedBandwidthBps, minimumBandwidthBps, delayMs); - scheduleUpload(delayMs, latency); // reschedule uploadAsync to run again 1000 ms later + proposedBandwidthBps, minimumBandwidthBps, delayMs); + scheduleUpload(delayMs, latency); // reschedule uploadAsync to run again 1000 ms later return; } } @@ -193,7 +190,7 @@ namespace ARIASDK_NS_BEGIN { LOG_TRACE("Scheduling upload in %d ms", nextUploadInMs); EventLatency proposed = calculateNewPriority(); - scheduleUpload(nextUploadInMs, proposed); // reschedule uploadAsync again + scheduleUpload(nextUploadInMs, proposed); // reschedule uploadAsync again } } @@ -248,21 +245,19 @@ namespace ARIASDK_NS_BEGIN void TransmissionPolicyManager::handleFinishAllUploads() { pauseAllUploads(); - allUploadsFinished(); // calls stats.onStop >> this->flushTaskDispatcher; + allUploadsFinished(); // calls stats.onStop >> this->flushTaskDispatcher; } void TransmissionPolicyManager::handleEventArrived(IncomingEventContextPtr const& event) { - if (m_isPaused) - { + if (m_isPaused) { return; } bool forceTimerRestart = false; /* This logic needs to be revised: one event in a dedicated HTTP post is wasteful! */ // Initiate upload right away - if (event->record.latency > EventLatency_RealTime) - { + if (event->record.latency > EventLatency_RealTime) { EventsUploadContextPtr ctx = new EventsUploadContext(); ctx->requestedMinLatency = event->record.latency; addUpload(ctx); @@ -356,5 +351,5 @@ namespace ARIASDK_NS_BEGIN finishUpload(ctx, -1); } -} -ARIASDK_NS_END + +} ARIASDK_NS_END diff --git a/lib/tpm/TransmissionPolicyManager.hpp b/lib/tpm/TransmissionPolicyManager.hpp index e6616edab..5da1eed6c 100644 --- a/lib/tpm/TransmissionPolicyManager.hpp +++ b/lib/tpm/TransmissionPolicyManager.hpp @@ -18,6 +18,14 @@ #include #include +// This macro allows to specify max upload task cancellation wait time at compile-time, +// addressing the case when a task that we are trying to cancel is currently running. +// Default value: 500ms - sufficient for upload scheduler/batcher task to finish. +// Alternate value: UINT64_MAX - for infinite wait until the task is completed. +#ifndef UPLOAD_TASK_CANCEL_TIME_MS +#define UPLOAD_TASK_CANCEL_TIME_MS 500 +#endif + namespace ARIASDK_NS_BEGIN { class TransmissionPolicyManager @@ -68,7 +76,6 @@ namespace ARIASDK_NS_BEGIN { std::atomic m_isPaused; std::atomic m_isUploadScheduled; uint64_t m_scheduledUploadTime; - std::mutex m_scheduledUploadMutex; PAL::DeferredCallbackHandle m_scheduledUpload; bool m_scheduledUploadAborted; @@ -119,7 +126,7 @@ namespace ARIASDK_NS_BEGIN { /// bool cancelUploadTask() { - uint64_t cancelWaitTimeMs = (m_scheduledUploadAborted) ? UINT64_MAX : 0; + uint64_t cancelWaitTimeMs = (m_scheduledUploadAborted) ? UPLOAD_TASK_CANCEL_TIME_MS : 0; bool result = m_scheduledUpload.Cancel(cancelWaitTimeMs); m_isUploadScheduled.exchange(false); return result; diff --git a/tests/functests/APITest.cpp b/tests/functests/APITest.cpp index c1f6357e9..113cc0e2c 100644 --- a/tests/functests/APITest.cpp +++ b/tests/functests/APITest.cpp @@ -419,7 +419,7 @@ TEST(APITest, LogManager_KilledEventsAreDropped) EXPECT_EQ(MAX_ITERATIONS, debugListener.numLogged); // TODO: it is possible that collector would return 503 here, in that case we may not get the 'kill-tokens' hint. // If it ever happens, the test would fail because the second iteration might try to upload. - EXPECT_EQ(1, debugListener.numHttpError); + EXPECT_EQ(1u, debugListener.numHttpError); debugListener.numCached = 0; } if (i == 1) @@ -427,12 +427,12 @@ TEST(APITest, LogManager_KilledEventsAreDropped) // At this point we should get the error response from collector because we ingested with invalid tokens. // Collector should have also asked us to ban that token... Check the counts EXPECT_EQ(2 * MAX_ITERATIONS, debugListener.numLogged); - EXPECT_EQ(0, debugListener.numCached); + EXPECT_EQ(0u, debugListener.numCached); EXPECT_EQ(MAX_ITERATIONS, debugListener.numDropped); } } LogManager::FlushAndTeardown(); - EXPECT_EQ(0, debugListener.numCached); + EXPECT_EQ(0u, debugListener.numCached); debugListener.printStats(); removeAllListeners(debugListener); } @@ -473,7 +473,7 @@ TEST(APITest, LogManager_Initialize_DebugEventListener) debugListener.storageFullPct = 0; LogManager::Initialize(TEST_TOKEN, configuration); LogManager::FlushAndTeardown(); - EXPECT_EQ(debugListener.storageFullPct.load(), 0); + EXPECT_EQ(debugListener.storageFullPct.load(), 0u); } debugListener.numCached = 0; @@ -489,12 +489,12 @@ TEST(APITest, LogManager_Initialize_DebugEventListener) result->LogEvent(eventToLog); // Check the counts EXPECT_EQ(MAX_ITERATIONS, debugListener.numLogged); - EXPECT_EQ(0, debugListener.numDropped); - EXPECT_EQ(0, debugListener.numReject); + EXPECT_EQ(0u, debugListener.numDropped); + EXPECT_EQ(0u, debugListener.numReject); LogManager::UploadNow(); // Try to upload whatever we got PAL::sleep(1000); // Give enough time to upload at least one event - EXPECT_NE(0, debugListener.numSent); // Some posts must succeed within 500ms + EXPECT_NE(0u, debugListener.numSent); // Some posts must succeed within 500ms LogManager::PauseTransmission(); // There could still be some pending at this point LogManager::Flush(); // Save all pending to disk @@ -767,7 +767,7 @@ TEST(APITest, C_API_Test) std::string guidStr2 = "01020304-0506-0708-090a-0b0c0d0e0f00"; ASSERT_STRCASEEQ(guidStr.c_str(), guidStr2.c_str()); // Verify time - ASSERT_EQ(record.data[0].properties["timeKey"].longValue, ticks.ticks); + ASSERT_EQ(record.data[0].properties["timeKey"].longValue, (int64_t)ticks.ticks); }; evt_handle_t handle = evt_open(config); @@ -788,7 +788,7 @@ TEST(APITest, C_API_Test) { evt_log(handle, event); } - EXPECT_EQ(totalEvents, 5); + EXPECT_EQ(totalEvents, 5u); evt_flush(handle); evt_upload(handle); @@ -850,7 +850,7 @@ TEST(APITest, UTC_Callback_Test) std::string guidStr2 = "01020304-0506-0708-090a-0b0c0d0e0f00"; ASSERT_STRCASEEQ(guidStr.c_str(), guidStr2.c_str()); // Verify time - ASSERT_EQ(record.data[0].properties["timeKey"].longValue, ticks.ticks); + ASSERT_EQ(record.data[0].properties["timeKey"].longValue, (int64_t)ticks.ticks); // Transform to JSON and print std::string s; @@ -910,7 +910,7 @@ TEST(APITest, Pii_DROP_Test) return; } - ASSERT_EQ(record.extProtocol[0].ticketKeys.size(), 0); + ASSERT_EQ(record.extProtocol[0].ticketKeys.size(), 0ul); // more events with random device id EXPECT_STRNE(record.extDevice[0].localId.c_str(), realDeviceId.c_str()); EXPECT_STREQ(record.extDevice[0].authId.c_str(), ""); @@ -954,7 +954,7 @@ TEST(APITest, Pii_DROP_Test) } LogManager::FlushAndTeardown(); - ASSERT_EQ(totalEvents, 4); + ASSERT_EQ(totalEvents, 4u); LogManager::RemoveEventListener(EVT_LOG_EVENT, debugListener); } diff --git a/tests/functests/BasicFuncTests.cpp b/tests/functests/BasicFuncTests.cpp index 3a59920d6..0ac04e290 100644 --- a/tests/functests/BasicFuncTests.cpp +++ b/tests/functests/BasicFuncTests.cpp @@ -1,28 +1,29 @@ +// clang-format off // Copyright (c) Microsoft. All rights reserved. #include "mat/config.h" #ifdef HAVE_MAT_DEFAULT_HTTP_CLIENT #ifndef WIN32_LEAN_AND_MEAN -#define WIN32_LEAN_AND_MEAN // Exclude rarely-used stuff from Windows headers +#define WIN32_LEAN_AND_MEAN // Exclude rarely-used stuff from Windows headers #endif -#include "api/LogManagerImpl.hpp" #include "common/Common.hpp" #include "common/HttpServer.hpp" #include "utils/Utils.hpp" +#include "api/LogManagerImpl.hpp" -#include "LogManager.hpp" #include "bond/All.hpp" -#include "bond/generated/CsProtocol_readers.hpp" #include "bond/generated/CsProtocol_types.hpp" +#include "bond/generated/CsProtocol_readers.hpp" +#include "LogManager.hpp" #include "CompliantByDefaultFilterApi.hpp" -#include +#include #include -#include +#include #include -#include #include +#include #include #include "PayloadDecoder.hpp" @@ -89,40 +90,40 @@ ARIASDK_NS_END char const* const TEST_STORAGE_FILENAME = "BasicFuncTests.db"; // 1DSCppSdktest sandbox key -#define TEST_TOKEN "7c8b1796cbc44bd5a03803c01c2b9d61-b6e370dd-28d9-4a52-9556-762543cf7aa7-6991" +#define TEST_TOKEN "7c8b1796cbc44bd5a03803c01c2b9d61-b6e370dd-28d9-4a52-9556-762543cf7aa7-6991" -#define KILLED_TOKEN "deadbeefdeadbeefdeadbeefdeadbeef-c2d379e0-4408-4325-9b4d-2a7d78131e14-7322" -#define HTTP_PORT 19000 +#define KILLED_TOKEN "deadbeefdeadbeefdeadbeefdeadbeef-c2d379e0-4408-4325-9b4d-2a7d78131e14-7322" +#define HTTP_PORT 19000 #undef LOCKGUARD -#define LOCKGUARD(macro_mutex) std::lock_guard TOKENPASTE2(__guard_, __LINE__)(macro_mutex); +#define LOCKGUARD(macro_mutex) std::lock_guard TOKENPASTE2(__guard_, __LINE__) (macro_mutex); class HttpPostListener : public DebugEventListener { - public: - virtual void OnDebugEvent(DebugEvent& evt) +public: + virtual void OnDebugEvent(DebugEvent &evt) { static unsigned seq = 0; switch (evt.type) { case EVT_HTTP_OK: - { - seq++; - std::string out; - std::vector reqBody((unsigned char*)evt.data, (unsigned char*)(evt.data) + evt.size); - MAT::exporters::DecodeRequest(reqBody, out, false); - printf(">>>> REQUEST [%u]:%s\n", seq, out.c_str()); - } - break; + { + seq++; + std::string out; + std::vector reqBody((unsigned char *)evt.data, (unsigned char *)(evt.data) + evt.size); + MAT::exporters::DecodeRequest(reqBody, out, false); + printf(">>>> REQUEST [%u]:%s\n", seq, out.c_str()); + } + break; default: break; }; }; }; class BasicFuncTests : public ::testing::Test, - public HttpServer::Callback + public HttpServer::Callback { - protected: - std::mutex mtx_requests; +protected: + std::mutex mtx_requests; std::vector receivedRequests; std::string serverAddress; HttpServer server; @@ -135,11 +136,12 @@ class BasicFuncTests : public ::testing::Test, std::condition_variable cv_gotEvents; std::mutex cv_m; +public: - public: BasicFuncTests() : - isSetup(false), - isRunning(false){}; + isSetup(false) , + isRunning(false) + {}; virtual void SetUp() override { @@ -155,7 +157,7 @@ class BasicFuncTests : public ::testing::Test, server.addHandler("/simple/", *this); server.addHandler("/slow/", *this); server.addHandler("/503/", *this); - server.setKeepalive(false); // This test doesn't work well with keep-alive enabled + server.setKeepalive(false); // This test doesn't work well with keep-alive enabled server.start(); isRunning = true; } @@ -191,20 +193,20 @@ class BasicFuncTests : public ::testing::Test, configuration[CFG_INT_RAM_QUEUE_SIZE] = 4096 * 20; configuration[CFG_STR_CACHE_FILE_PATH] = TEST_STORAGE_FILENAME; - configuration[CFG_INT_MAX_TEARDOWN_TIME] = 2; // 2 seconds wait on shutdown + configuration[CFG_INT_MAX_TEARDOWN_TIME] = 2; // 2 seconds wait on shutdown configuration[CFG_STR_COLLECTOR_URL] = serverAddress.c_str(); - configuration["http"]["compress"] = false; // disable compression for now - configuration["stats"]["interval"] = 30 * 60; // 30 mins + configuration["http"]["compress"] = false; // disable compression for now + configuration["stats"]["interval"] = 30 * 60; // 30 mins configuration["name"] = __FILE__; configuration["version"] = "1.0.0"; - configuration["config"] = {{"host", __FILE__}}; // Host instance + configuration["config"] = { { "host", __FILE__ } }; // Host instance LogManager::Initialize(TEST_TOKEN, configuration); - LogManager::SetLevelFilter(DIAG_LEVEL_DEFAULT, {DIAG_LEVEL_DEFAULT_MIN, DIAG_LEVEL_DEFAULT_MAX}); + LogManager::SetLevelFilter(DIAG_LEVEL_DEFAULT, { DIAG_LEVEL_DEFAULT_MIN, DIAG_LEVEL_DEFAULT_MAX }); LogManager::ResumeTransmission(); - logger = LogManager::GetLogger(TEST_TOKEN, "source1"); + logger = LogManager::GetLogger(TEST_TOKEN, "source1"); logger2 = LogManager::GetLogger(TEST_TOKEN, "source2"); } @@ -215,13 +217,11 @@ class BasicFuncTests : public ::testing::Test, virtual int onHttpRequest(HttpServer::Request const& request, HttpServer::Response& response) override { - if (request.uri.compare(0, 5, "/503/") == 0) - { + if (request.uri.compare(0, 5, "/503/") == 0) { return 503; } - if (request.uri.compare(0, 6, "/slow/") == 0) - { + if (request.uri.compare(0, 6, "/slow/") == 0) { PAL::sleep(static_cast(request.content.size() / DELAY_FACTOR_FOR_SERVER)); } @@ -251,7 +251,7 @@ class BasicFuncTests : public ::testing::Test, unsigned receivedEvents = 0; auto start = PAL::getUtcSystemTimeMs(); size_t lastIdx = 0; - while (((PAL::getUtcSystemTimeMs() - start) < (1000 * timeOutSec)) && (receivedEvents != expected_count)) + while ( ((PAL::getUtcSystemTimeMs()-start)<(1000* timeOutSec)) && (receivedEvents!=expected_count) ) { /* Give time for our friendly HTTP server thread to processs incoming request */ std::this_thread::yield(); @@ -266,7 +266,7 @@ class BasicFuncTests : public ::testing::Test, { auto request = receivedRequests.at(index); auto payload = decodeRequest(request, false); - receivedEvents += (unsigned)payload.size(); + receivedEvents+= (unsigned)payload.size(); } lastIdx = size; } @@ -321,6 +321,7 @@ class BasicFuncTests : public ::testing::Test, EXPECT_THAT(bond_lite::Deserialize(reader, result), true); data += index - 1; vector.push_back(result); + } return vector; @@ -373,7 +374,7 @@ class BasicFuncTests : public ::testing::Test, } case ::CsProtocol::ValueGuid: { - uint8_t guid_bytes[16] = {0}; + uint8_t guid_bytes[16] = { 0 }; prop.second.as_guid.to_bytes(guid_bytes); std::vector guid = std::vector(guid_bytes, guid_bytes + sizeof(guid_bytes) / sizeof(guid_bytes[0])); @@ -436,7 +437,7 @@ class BasicFuncTests : public ::testing::Test, EXPECT_THAT(vectror.size(), prop.second.as_guidArray->size()); for (size_t index = 0; index < prop.second.as_guidArray->size(); index++) { - uint8_t guid_bytes[16] = {0}; + uint8_t guid_bytes[16] = { 0 }; prop.second.as_guidArray->at(index).to_bytes(guid_bytes); std::vector guid = std::vector(guid_bytes, guid_bytes + sizeof(guid_bytes) / sizeof(guid_bytes[0])); @@ -487,11 +488,11 @@ class BasicFuncTests : public ::testing::Test, std::vector result; if (receivedRequests.size()) { - for (auto& request : receivedRequests) + for (auto &request : receivedRequests) { // TODO: [MG] - add compression support auto payload = decodeRequest(request, false); - for (auto& record : payload) + for (auto &record : payload) { result.push_back(std::move(record)); } @@ -507,11 +508,11 @@ class BasicFuncTests : public ::testing::Test, result.name = ""; if (receivedRequests.size()) { - for (auto& request : receivedRequests) + for (auto &request : receivedRequests) { // TODO: [MG] - add compression support auto payload = decodeRequest(request, false); - for (auto& record : payload) + for (auto &record : payload) { if (record.name == name) { @@ -525,6 +526,7 @@ class BasicFuncTests : public ::testing::Test, } }; + TEST_F(BasicFuncTests, doNothing) { } @@ -537,7 +539,7 @@ TEST_F(BasicFuncTests, sendOneEvent_immediatelyStop) event.SetProperty("property", "value"); logger->LogEvent(event); FlushAndTeardown(); - EXPECT_GE(receivedRequests.size(), (size_t)1); // at least 1 HTTP request with customer payload and stats + EXPECT_GE(receivedRequests.size(), (size_t)1); // at least 1 HTTP request with customer payload and stats } TEST_F(BasicFuncTests, sendNoPriorityEvents) @@ -564,9 +566,10 @@ TEST_F(BasicFuncTests, sendNoPriorityEvents) if (receivedRequests.size() >= 1) { - verifyEvent(event, find("first_event")); + verifyEvent(event, find("first_event")); verifyEvent(event2, find("second_event")); } + } TEST_F(BasicFuncTests, sendSamePriorityNormalEvents) @@ -604,7 +607,7 @@ TEST_F(BasicFuncTests, sendSamePriorityNormalEvents) logger->LogEvent(event2); waitForEvents(2, 3); - for (const auto& evt : {event, event2}) + for (const auto &evt : { event, event2 }) { verifyEvent(evt, find(evt.GetName())); } @@ -640,6 +643,7 @@ TEST_F(BasicFuncTests, sendDifferentPriorityEvents) std::fill(gvector.begin() + 3, gvector.end() - 2, GUID_t("00000000-0000-0000-0000-000000000000")); event.SetProperty("property4", gvector); + logger->LogEvent(event); EventProperties event2("second_event"); @@ -649,12 +653,13 @@ TEST_F(BasicFuncTests, sendDifferentPriorityEvents) event2.SetProperty("pii_property", "pii_value", PiiKind_Identity); event2.SetProperty("cc_property", "cc_value", CustomerContentKind_GenericData); + logger->LogEvent(event2); LogManager::UploadNow(); waitForEvents(1, 3); - for (const auto& evt : {event, event2}) + for (const auto &evt : { event, event2 }) { verifyEvent(evt, find(evt.GetName())); } @@ -699,12 +704,13 @@ TEST_F(BasicFuncTests, sendMultipleTenantsTogether) LogManager::UploadNow(); waitForEvents(1, 3); - for (const auto& evt : {event1, event2}) + for (const auto &evt : { event1, event2 }) { verifyEvent(evt, find(evt.GetName())); } FlushAndTeardown(); + } TEST_F(BasicFuncTests, configDecorations) @@ -727,7 +733,7 @@ TEST_F(BasicFuncTests, configDecorations) LogManager::UploadNow(); waitForEvents(2, 5); - for (const auto& evt : {event1, event2, event3, event4}) + for (const auto &evt : { event1, event2, event3, event4 }) { verifyEvent(evt, find(evt.GetName())); } @@ -764,7 +770,7 @@ TEST_F(BasicFuncTests, restartRecoversEventsFromStorage) LogManager::UploadNow(); // 1st request for realtime event - waitForEvents(3, 7); // start, first_event, second_event, ongoing, stop, start, fooEvent + waitForEvents(3, 7); // start, first_event, second_event, ongoing, stop, start, fooEvent EXPECT_GE(receivedRequests.size(), (size_t)1); if (receivedRequests.size() != 0) { @@ -785,7 +791,7 @@ TEST_F(BasicFuncTests, restartRecoversEventsFromStorage) */ } -#if 0 // FIXME: 1445871 [v3][1DS] Offline storage size may exceed configured limit +#if 0 // FIXME: 1445871 [v3][1DS] Offline storage size may exceed configured limit TEST_F(BasicFuncTests, storageFileSizeDoesntExceedConfiguredSize) { CleanStorage(); @@ -867,18 +873,18 @@ TEST_F(BasicFuncTests, sendMetaStatsOnStart) FlushAndTeardown(); auto r1 = records(); - ASSERT_EQ(r1.size(), size_t{0}); + ASSERT_EQ(r1.size(), size_t { 0 }); // Check Initialize(); - LogManager::ResumeTransmission(); // ? + LogManager::ResumeTransmission(); // ? LogManager::UploadNow(); PAL::sleep(2000); auto r2 = records(); - ASSERT_GE(r2.size(), (size_t)4); // (start + stop) + (2 events + start) + ASSERT_GE(r2.size(), (size_t)4); // (start + stop) + (2 events + start) - for (const auto& evt : {event1, event2}) + for (const auto &evt : { event1, event2 }) { verifyEvent(evt, find(evt.GetName())); } @@ -890,7 +896,7 @@ TEST_F(BasicFuncTests, DiagLevelRequiredOnly_OneEventWithoutLevelOneWithButNotAl { CleanStorage(); Initialize(); - LogManager::SetLevelFilter(DIAG_LEVEL_OPTIONAL, {DIAG_LEVEL_REQUIRED}); + LogManager::SetLevelFilter(DIAG_LEVEL_OPTIONAL, { DIAG_LEVEL_REQUIRED }); EventProperties eventWithoutLevel("EventWithoutLevel"); logger->LogEvent(eventWithoutLevel); @@ -905,7 +911,7 @@ TEST_F(BasicFuncTests, DiagLevelRequiredOnly_OneEventWithoutLevelOneWithButNotAl LogManager::UploadNow(); waitForEvents(1 /*timeout*/, 2 /*expected count*/); // Start and EventWithAllowedLevel - ASSERT_EQ(records().size(), static_cast(2)); // Start and EventWithAllowedLevel + ASSERT_EQ(records().size(), static_cast(2)); // Start and EventWithAllowedLevel verifyEvent(eventWithAllowedLevel, find(eventWithAllowedLevel.GetName())); @@ -939,34 +945,34 @@ TEST_F(BasicFuncTests, DiagLevelRequiredOnly_SendTwoEventsUpdateAllowedLevelsSen CleanStorage(); Initialize(); - LogManager::SetLevelFilter(DIAG_LEVEL_OPTIONAL, {DIAG_LEVEL_REQUIRED}); + LogManager::SetLevelFilter(DIAG_LEVEL_OPTIONAL, { DIAG_LEVEL_REQUIRED }); SendEventWithOptionalThenRequired(logger); - LogManager::SetLevelFilter(DIAG_LEVEL_OPTIONAL, {DIAG_LEVEL_OPTIONAL, DIAG_LEVEL_REQUIRED}); + LogManager::SetLevelFilter(DIAG_LEVEL_OPTIONAL, { DIAG_LEVEL_OPTIONAL, DIAG_LEVEL_REQUIRED }); SendEventWithOptionalThenRequired(logger); LogManager::UploadNow(); - waitForEvents(2 /*timeout*/, 4 /*expected count*/); // Start and EventWithAllowedLevel + waitForEvents(2 /*timeout*/, 4 /*expected count*/); // Start and EventWithAllowedLevel auto sentRecords = records(); - ASSERT_EQ(sentRecords.size(), static_cast(4)); // Start and EventWithAllowedLevel - ASSERT_EQ(GetEventsWithName("EventWithOptionalLevel", sentRecords).size(), size_t{1}); - ASSERT_EQ(GetEventsWithName("EventWithRequiredLevel", sentRecords).size(), size_t{2}); + ASSERT_EQ(sentRecords.size(), static_cast(4)); // Start and EventWithAllowedLevel + ASSERT_EQ(GetEventsWithName("EventWithOptionalLevel", sentRecords).size(), size_t{ 1 }); + ASSERT_EQ(GetEventsWithName("EventWithRequiredLevel", sentRecords).size(), size_t{ 2 }); FlushAndTeardown(); } class RequestMonitor : public DebugEventListener { - const size_t IDX_OK = 0; - const size_t IDX_ERR = 1; + const size_t IDX_OK = 0; + const size_t IDX_ERR = 1; const size_t IDX_ABRT = 2; std::atomic counts[3]; - public: - RequestMonitor() : - DebugEventListener() +public: + + RequestMonitor() : DebugEventListener() { reset(); } @@ -978,7 +984,7 @@ class RequestMonitor : public DebugEventListener counts[IDX_ABRT] = 0; } - virtual void OnDebugEvent(DebugEvent& evt) + virtual void OnDebugEvent(DebugEvent &evt) { switch (evt.type) { @@ -1008,17 +1014,16 @@ class RequestMonitor : public DebugEventListener } }; -class KillSwitchListener : public DebugEventListener -{ - public: - std::atomic numLogged; - std::atomic numSent; - std::atomic numDropped; - std::atomic numReject; - std::atomic numHttpError; - std::atomic numHttpOK; - std::atomic numHttpFailure; - std::atomic numCached; +class KillSwitchListener : public DebugEventListener { +public : + std::atomic numLogged; + std::atomic numSent; + std::atomic numDropped; + std::atomic numReject; + std::atomic numHttpError; + std::atomic numHttpOK; + std::atomic numHttpFailure; + std::atomic numCached; KillSwitchListener() : numLogged(0), @@ -1028,48 +1033,45 @@ class KillSwitchListener : public DebugEventListener numHttpError(0), numHttpOK(0), numHttpFailure(0), - numCached(0) + numCached(0) { } - virtual void OnDebugEvent(DebugEvent& evt) - { - switch (evt.type) - { - case EVT_LOG_SESSION: - numLogged++; - break; - case EVT_REJECTED: - numReject++; - break; - case EVT_ADDED: - break; - /* Event counts below would never overflow the size of unsigned int */ - case EVT_CACHED: - numCached += (unsigned int)evt.param1; - break; - case EVT_DROPPED: - numDropped += (unsigned int)evt.param1; - break; - case EVT_SENT: - numSent += (unsigned int)evt.param1; - break; - case EVT_HTTP_FAILURE: - numHttpFailure++; - break; - case EVT_HTTP_ERROR: - numHttpError++; - break; - case EVT_HTTP_OK: - numHttpOK++; - break; - case EVT_UNKNOWN: - default: + virtual void OnDebugEvent(DebugEvent &evt) { + switch (evt.type) { + case EVT_LOG_SESSION: + numLogged++; + break; + case EVT_REJECTED: + numReject++; + break; + case EVT_ADDED: + break; + /* Event counts below would never overflow the size of unsigned int */ + case EVT_CACHED: + numCached += (unsigned int)evt.param1; + break; + case EVT_DROPPED: + numDropped += (unsigned int)evt.param1; + break; + case EVT_SENT: + numSent += (unsigned int)evt.param1; + break; + case EVT_HTTP_FAILURE: + numHttpFailure++; + break; + case EVT_HTTP_ERROR: + numHttpError++; + break; + case EVT_HTTP_OK: + numHttpOK++; + break; + case EVT_UNKNOWN: + default: break; }; } - void printStats() - { + void printStats(){ std::cerr << "[ ] numLogged = " << numLogged << std::endl; std::cerr << "[ ] numSent = " << numSent << std::endl; std::cerr << "[ ] numDropped = " << numDropped << std::endl; @@ -1081,8 +1083,7 @@ class KillSwitchListener : public DebugEventListener } }; -void addListeners(DebugEventListener& listener) -{ +void addListeners(DebugEventListener &listener) { LogManager::AddEventListener(DebugEventType::EVT_LOG_SESSION, listener); LogManager::AddEventListener(DebugEventType::EVT_REJECTED, listener); LogManager::AddEventListener(DebugEventType::EVT_SENT, listener); @@ -1093,8 +1094,7 @@ void addListeners(DebugEventListener& listener) LogManager::AddEventListener(DebugEventType::EVT_CACHED, listener); } -void removeListeners(DebugEventListener& listener) -{ +void removeListeners(DebugEventListener &listener) { LogManager::RemoveEventListener(DebugEventType::EVT_LOG_SESSION, listener); LogManager::RemoveEventListener(DebugEventType::EVT_REJECTED, listener); LogManager::RemoveEventListener(DebugEventType::EVT_SENT, listener); @@ -1117,14 +1117,14 @@ TEST_F(BasicFuncTests, killSwitchWorks) configuration[CFG_INT_RAM_QUEUE_SIZE] = 4096 * 20; configuration[CFG_STR_CACHE_FILE_PATH] = TEST_STORAGE_FILENAME; - configuration[CFG_INT_MAX_TEARDOWN_TIME] = 2; // 2 seconds wait on shutdown + configuration[CFG_INT_MAX_TEARDOWN_TIME] = 2; // 2 seconds wait on shutdown configuration[CFG_STR_COLLECTOR_URL] = serverAddress.c_str(); - configuration["http"]["compress"] = false; // disable compression for now - configuration["stats"]["interval"] = 30 * 60; // 30 mins + configuration["http"]["compress"] = false; // disable compression for now + configuration["stats"]["interval"] = 30 * 60; // 30 mins configuration["name"] = __FILE__; configuration["version"] = "1.0.0"; - configuration["config"] = {{"host", __FILE__}}; // Host instance + configuration["config"] = { { "host", __FILE__ } }; // Host instance // set the killed token on the server server.setKilledToken(KILLED_TOKEN, 6384); @@ -1132,15 +1132,13 @@ TEST_F(BasicFuncTests, killSwitchWorks) addListeners(listener); // Log 100 events from valid and invalid 4 times int repetitions = 4; - for (int i = 0; i < repetitions; i++) - { + for (int i = 0; i < repetitions; i++) { // Initialize the logger for the valid token and log 100 events LogManager::Initialize(TEST_TOKEN, configuration); LogManager::ResumeTransmission(); auto myLogger = LogManager::GetLogger(TEST_TOKEN, "killed"); int numIterations = 100; - while (numIterations--) - { + while (numIterations--) { EventProperties event1("fooEvent"); event1.SetProperty("property", "value"); myLogger->LogEvent(event1); @@ -1150,8 +1148,7 @@ TEST_F(BasicFuncTests, killSwitchWorks) LogManager::ResumeTransmission(); myLogger = LogManager::GetLogger(KILLED_TOKEN, "killed"); numIterations = 100; - while (numIterations--) - { + while (numIterations--) { EventProperties event2("failEvent"); event2.SetProperty("property", "value"); myLogger->LogEvent(event2); @@ -1166,8 +1163,7 @@ TEST_F(BasicFuncTests, killSwitchWorks) LogManager::ResumeTransmission(); auto myLogger = LogManager::GetLogger(TEST_TOKEN, "killed"); int numIterations = 100; - while (numIterations--) - { + while (numIterations--) { EventProperties event1("fooEvent"); event1.SetProperty("property", "value"); myLogger->LogEvent(event1); @@ -1177,14 +1173,13 @@ TEST_F(BasicFuncTests, killSwitchWorks) LogManager::ResumeTransmission(); myLogger = LogManager::GetLogger(KILLED_TOKEN, "killed"); numIterations = 100; - while (numIterations--) - { + while (numIterations--) { EventProperties event2("failEvent"); event2.SetProperty("property", "value"); myLogger->LogEvent(event2); } // Expect all events to be dropped - EXPECT_EQ(uint32_t{100}, listener.numDropped); + EXPECT_EQ(uint32_t { 100 }, listener.numDropped); LogManager::FlushAndTeardown(); listener.printStats(); @@ -1192,7 +1187,7 @@ TEST_F(BasicFuncTests, killSwitchWorks) server.clearKilledTokens(); } -TEST_F(BasicFuncTests, killIsTemporary) +TEST_F(BasicFuncTests, killIsTemporary) { CleanStorage(); // Create the configuration to send to fake server @@ -1204,30 +1199,28 @@ TEST_F(BasicFuncTests, killIsTemporary) configuration[CFG_INT_RAM_QUEUE_SIZE] = 4096 * 20; configuration[CFG_STR_CACHE_FILE_PATH] = TEST_STORAGE_FILENAME; - configuration[CFG_INT_MAX_TEARDOWN_TIME] = 2; // 2 seconds wait on shutdown + configuration[CFG_INT_MAX_TEARDOWN_TIME] = 2; // 2 seconds wait on shutdown configuration[CFG_STR_COLLECTOR_URL] = serverAddress.c_str(); - configuration["http"]["compress"] = false; // disable compression for now - configuration["stats"]["interval"] = 30 * 60; // 30 mins + configuration["http"]["compress"] = false; // disable compression for now + configuration["stats"]["interval"] = 30 * 60; // 30 mins configuration["name"] = __FILE__; configuration["version"] = "1.0.0"; - configuration["config"] = {{"host", __FILE__}}; // Host instance - + configuration["config"] = { { "host", __FILE__ } }; // Host instance + // set the killed token on the server server.setKilledToken(KILLED_TOKEN, 10); KillSwitchListener listener; addListeners(listener); // Log 100 events from valid and invalid 4 times int repetitions = 4; - for (int i = 0; i < repetitions; i++) - { + for (int i = 0; i < repetitions; i++) { // Initialize the logger for the valid token and log 100 events LogManager::Initialize(TEST_TOKEN, configuration); LogManager::ResumeTransmission(); auto myLogger = LogManager::GetLogger(TEST_TOKEN, "killed"); int numIterations = 100; - while (numIterations--) - { + while (numIterations--) { EventProperties event1("fooEvent"); event1.SetProperty("property", "value"); myLogger->LogEvent(event1); @@ -1237,8 +1230,7 @@ TEST_F(BasicFuncTests, killIsTemporary) LogManager::ResumeTransmission(); myLogger = LogManager::GetLogger(KILLED_TOKEN, "killed"); numIterations = 100; - while (numIterations--) - { + while (numIterations--) { EventProperties event2("failEvent"); event2.SetProperty("property", "value"); myLogger->LogEvent(event2); @@ -1255,8 +1247,7 @@ TEST_F(BasicFuncTests, killIsTemporary) LogManager::ResumeTransmission(); auto myLogger = LogManager::GetLogger(TEST_TOKEN, "killed"); int numIterations = 100; - while (numIterations--) - { + while (numIterations--) { EventProperties event1("fooEvent"); event1.SetProperty("property", "value"); myLogger->LogEvent(event1); @@ -1266,14 +1257,13 @@ TEST_F(BasicFuncTests, killIsTemporary) LogManager::ResumeTransmission(); myLogger = LogManager::GetLogger(KILLED_TOKEN, "killed"); numIterations = 100; - while (numIterations--) - { + while (numIterations--) { EventProperties event2("failEvent"); event2.SetProperty("property", "value"); myLogger->LogEvent(event2); } // Expect to 0 events to be dropped - EXPECT_EQ(uint32_t{0}, listener.numDropped); + EXPECT_EQ(uint32_t { 0 }, listener.numDropped); LogManager::FlushAndTeardown(); listener.printStats(); @@ -1291,7 +1281,8 @@ TEST_F(BasicFuncTests, sendManyRequestsAndCancel) auto eventsList = { DebugEventType::EVT_HTTP_OK, DebugEventType::EVT_HTTP_ERROR, - DebugEventType::EVT_HTTP_FAILURE}; + DebugEventType::EVT_HTTP_FAILURE + }; // Add event listeners for (auto evt : eventsList) { @@ -1300,7 +1291,7 @@ TEST_F(BasicFuncTests, sendManyRequestsAndCancel) for (size_t i = 0; i < 20; i++) { - auto& configuration = LogManager::GetLogConfiguration(); + auto &configuration = LogManager::GetLogConfiguration(); configuration[CFG_INT_RAM_QUEUE_SIZE] = 4096 * 20; configuration[CFG_STR_CACHE_FILE_PATH] = TEST_STORAGE_FILENAME; configuration["http"]["compress"] = true; @@ -1440,10 +1431,11 @@ TEST_F(BasicFuncTests, raceBetweenUploadAndShutdownMultipleLogManagers) LogManagerB::RemoveEventListener(evt, listener); LogManagerA::RemoveEventListener(evt, listener); } + CleanStorage(); } #endif -#if 0 // XXX: [MG] - This test was never supposed to work! Because the URL is invalid, we won't get anything in receivedRequests +#if 0 // XXX: [MG] - This test was never supposed to work! Because the URL is invalid, we won't get anything in receivedRequests TEST_F(BasicFuncTests, networkProblemsDoNotDropEvents) { @@ -1478,7 +1470,7 @@ TEST_F(BasicFuncTests, networkProblemsDoNotDropEvents) } #endif -#if 0 // TODO: [MG] - re-enable this long-haul test +#if 0 // TODO: [MG] - re-enable this long-haul test TEST_F(BasicFuncTests, serverProblemsDropEventsAfterMaxRetryCount) { CleanStorage(); @@ -1530,4 +1522,4 @@ TEST_F(BasicFuncTests, serverProblemsDropEventsAfterMaxRetryCount) } } #endif -#endif // HAVE_MAT_DEFAULT_HTTP_CLIENT +#endif // HAVE_MAT_DEFAULT_HTTP_CLIENT diff --git a/tests/functests/BondDecoderTests.cpp b/tests/functests/BondDecoderTests.cpp index 85ae7ad54..da2f6ef40 100644 --- a/tests/functests/BondDecoderTests.cpp +++ b/tests/functests/BondDecoderTests.cpp @@ -45,7 +45,7 @@ void SendEvents(ILogger* pLogger, uint8_t eventCount, std::chrono::milliseconds { // Key-values { "strKey", "hello" }, - { "int64Key", 1L }, + { "int64Key", 1LL }, { "dblKey", 3.14 }, { "boolKey", false }, { "guidKey0", GUID_t("00000000-0000-0000-0000-000000000000") }, diff --git a/tests/functests/FuncTests.vcxproj b/tests/functests/FuncTests.vcxproj index e2d9cb77d..b75e8fb88 100644 --- a/tests/functests/FuncTests.vcxproj +++ b/tests/functests/FuncTests.vcxproj @@ -165,7 +165,7 @@ /machine:X86 %(AdditionalOptions) - pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) @@ -213,7 +213,7 @@ /machine:X86 %(AdditionalOptions) - pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) No %(IgnoreSpecificDefaultLibraries) @@ -260,7 +260,7 @@ /machine:X64 %(AdditionalOptions) - pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) @@ -309,7 +309,7 @@ /machine:ARM64 %(AdditionalOptions) - pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) @@ -357,7 +357,7 @@ /machine:X64 %(AdditionalOptions) - pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) Debug %(IgnoreSpecificDefaultLibraries) @@ -403,7 +403,7 @@ /machine:ARM64 %(AdditionalOptions) - pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) Debug %(IgnoreSpecificDefaultLibraries) diff --git a/tests/functests/FuncTests.vcxproj.filters b/tests/functests/FuncTests.vcxproj.filters index 053a42eca..e1e2bb090 100644 --- a/tests/functests/FuncTests.vcxproj.filters +++ b/tests/functests/FuncTests.vcxproj.filters @@ -1,74 +1,75 @@ - - - - - - - - mocks - - - common - - - - - - - - - mocks - - - mocks - - - mocks - - - mocks - - - mocks - - - mocks - - - common - - - mocks - - - mocks - - - mocks - - - mocks - - - common - - - common - - - mocks - - - - - - {785D8461-1CA4-30CA-9A64-692B8A3A06A7} - - - {F910581B-17A7-3DD0-AEFD-1B8C0A922D49} - - - {5f02135a-4d1e-496a-900a-e5ea8cfb222d} - - + + + + + + + + mocks + + + common + + + + + + + + + + mocks + + + mocks + + + mocks + + + mocks + + + mocks + + + mocks + + + common + + + mocks + + + mocks + + + mocks + + + mocks + + + common + + + common + + + mocks + + + + + + {785D8461-1CA4-30CA-9A64-692B8A3A06A7} + + + {F910581B-17A7-3DD0-AEFD-1B8C0A922D49} + + + {5f02135a-4d1e-496a-900a-e5ea8cfb222d} + + \ No newline at end of file diff --git a/tests/functests/LogSessionDataFuncTests.cpp b/tests/functests/LogSessionDataFuncTests.cpp index 26dcc2d25..f44e27ab2 100644 --- a/tests/functests/LogSessionDataFuncTests.cpp +++ b/tests/functests/LogSessionDataFuncTests.cpp @@ -84,7 +84,7 @@ TEST_F(LogSessionDataFuncTests, Constructor_ValidSessionFileExists_MembersSetToE ConstructSesFile(SessionFile, validSessionFirstTime, validSkuId); LogSessionData logSessionData{ SessionFileArgument }; - ASSERT_EQ(logSessionData.getSessionFirstTime(), 123456); + ASSERT_EQ(logSessionData.getSessionFirstTime(), 123456ull); ASSERT_EQ(logSessionData.getSessionSDKUid(), validSkuId); } @@ -95,7 +95,7 @@ TEST_F(LogSessionDataFuncTests, Constructor_InvalidSessionFileExists_MembersRege ConstructSesFile(SessionFile, invalidSessionFirstTime, validSkuId); LogSessionData logSessionData{ SessionFileArgument }; - ASSERT_NE(logSessionData.getSessionFirstTime(), 123456); + ASSERT_NE(logSessionData.getSessionFirstTime(), 123456ull); ASSERT_NE(logSessionData.getSessionSDKUid(), validSkuId); } From b1679b27ed599a6997b481b91d327ba6c3028b5b Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 5 Mar 2020 02:20:04 -0800 Subject: [PATCH 04/11] Update WorkerThread.cpp spelling and wording --- lib/pal/WorkerThread.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pal/WorkerThread.cpp b/lib/pal/WorkerThread.cpp index 7ca1320b7..cdda2f50f 100644 --- a/lib/pal/WorkerThread.cpp +++ b/lib/pal/WorkerThread.cpp @@ -128,8 +128,8 @@ namespace PAL_NS_BEGIN { } } /* Either waited long enough or the task is still executing. Return: - - true - if new item in progress is another new item - - false - if the item in progress is still the item we were waiting on + * true - if item in progress is different than item (other task) + * false - if item in progress is still the same (didn't wait long enough) */ return (m_itemInProgress != item); } From e142d4574e78469de45f61f3f5f0c6d7ef465e77 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 5 Mar 2020 02:23:07 -0800 Subject: [PATCH 05/11] Update TransmissionPolicyManager.cpp Spelling --- lib/tpm/TransmissionPolicyManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tpm/TransmissionPolicyManager.cpp b/lib/tpm/TransmissionPolicyManager.cpp index 146be6489..800d014f6 100644 --- a/lib/tpm/TransmissionPolicyManager.cpp +++ b/lib/tpm/TransmissionPolicyManager.cpp @@ -147,7 +147,7 @@ namespace ARIASDK_NS_BEGIN { LOCKGUARD(m_scheduledUploadMutex); if ((m_isPaused) || (m_scheduledUploadAborted)) { - LOG_TRACE("Paused, not uploading anything until resumed"); + LOG_TRACE("Paused or upload aborted: cancel pending upload task."); cancelUploadTask(); // If there is a pending upload task, kill it return; } From e187a9a401a9d138968f2c29323c5a2661bbc698 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 5 Mar 2020 02:26:15 -0800 Subject: [PATCH 06/11] Rename m_backoff_lock to m_backoffMutex --- lib/tpm/TransmissionPolicyManager.cpp | 6 +++--- lib/tpm/TransmissionPolicyManager.hpp | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/tpm/TransmissionPolicyManager.cpp b/lib/tpm/TransmissionPolicyManager.cpp index 146be6489..32dea93e0 100644 --- a/lib/tpm/TransmissionPolicyManager.cpp +++ b/lib/tpm/TransmissionPolicyManager.cpp @@ -40,7 +40,7 @@ namespace ARIASDK_NS_BEGIN { void TransmissionPolicyManager::checkBackoffConfigUpdate() { - LOCKGUARD(m_backoff_lock); + LOCKGUARD(m_backoffMutex); std::string config = m_config.GetUploadRetryBackoffConfig(); if (config != m_backoffConfig) { @@ -59,7 +59,7 @@ namespace ARIASDK_NS_BEGIN { void TransmissionPolicyManager::resetBackoff() { - LOCKGUARD(m_backoff_lock); + LOCKGUARD(m_backoffMutex); if (m_backoff) m_backoff->reset(); } @@ -67,7 +67,7 @@ namespace ARIASDK_NS_BEGIN { int TransmissionPolicyManager::increaseBackoff() { int delayMs = 0; - LOCKGUARD(m_backoff_lock); + LOCKGUARD(m_backoffMutex); checkBackoffConfigUpdate(); if (m_backoff) { diff --git a/lib/tpm/TransmissionPolicyManager.hpp b/lib/tpm/TransmissionPolicyManager.hpp index 5da1eed6c..3b081a074 100644 --- a/lib/tpm/TransmissionPolicyManager.hpp +++ b/lib/tpm/TransmissionPolicyManager.hpp @@ -68,7 +68,7 @@ namespace ARIASDK_NS_BEGIN { IRuntimeConfig& m_config; IBandwidthController* m_bandwidthController; - std::recursive_mutex m_backoff_lock; + std::recursive_mutex m_backoffMutex; std::string m_backoffConfig; // TODO: [MG] - move to config std::unique_ptr m_backoff; DeviceStateHandler m_deviceStateHandler; From bd31083a55de116aeabe93ba2101de9a0bd2fd0f Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 5 Mar 2020 14:05:10 -0800 Subject: [PATCH 07/11] Update ITaskDispatcher.hpp Eventhough we don't currently use tid anywhere, the intent was that it has to be static - sequentially incrementing across all tasks running under different log managers. --- lib/include/public/ITaskDispatcher.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/include/public/ITaskDispatcher.hpp b/lib/include/public/ITaskDispatcher.hpp index ffc6c36a8..60ef2f02c 100644 --- a/lib/include/public/ITaskDispatcher.hpp +++ b/lib/include/public/ITaskDispatcher.hpp @@ -25,7 +25,7 @@ namespace ARIASDK_NS_BEGIN /// static uint64_t GetNewTid() { - std::atomic lastTid; + static std::atomic lastTid; return lastTid.fetch_add(1); } From 350e5d895e88f1ff96607ed7130e8139c5901593 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Mon, 9 Mar 2020 13:50:38 -0700 Subject: [PATCH 08/11] Addressing code review concerns about a few possible race conditions --- .../cpp/SampleCppMini/SampleCppMini.vcxproj | 3152 +++++++++-------- lib/http/HttpClient_WinInet.cpp | 5 +- lib/include/public/LogManagerBase.hpp | 2 +- lib/offline/OfflineStorageHandler.cpp | 4 +- lib/pal/TaskDispatcher.hpp | 19 +- lib/pal/WorkerThread.cpp | 44 +- lib/tpm/TransmissionPolicyManager.cpp | 4 +- 7 files changed, 1633 insertions(+), 1597 deletions(-) diff --git a/examples/cpp/SampleCppMini/SampleCppMini.vcxproj b/examples/cpp/SampleCppMini/SampleCppMini.vcxproj index 97bd892ad..a2891ace7 100644 --- a/examples/cpp/SampleCppMini/SampleCppMini.vcxproj +++ b/examples/cpp/SampleCppMini/SampleCppMini.vcxproj @@ -1,1571 +1,1583 @@ - - - - - Debug.static - ARM64 - - - Debug.static - Win32 - - - Debug.static - x64 - - - Debug.vs2015.MT-sqlite - ARM64 - - - Debug.vs2015.MT-sqlite - Win32 - - - Debug.vs2015.MT-sqlite - x64 - - - Debug - ARM64 - - - Debug - Win32 - - - Release.static - ARM64 - - - Release.static - Win32 - - - Release.static - x64 - - - Release.vs2015.MT-sqlite - ARM64 - - - Release.vs2015.MT-sqlite - Win32 - - - Release.vs2015.MT-sqlite - x64 - - - Release - ARM64 - - - Release - Win32 - - - Debug - x64 - - - Release - x64 - - - - 15.0 - {86AC752C-5687-4377-841E-943D9BEEF361} - Win32Proj - SampleCppMini - 10.0.17763.0 - true - - - - Application - true - v141 - Unicode - Static - - - Application - true - v141 - Unicode - Static - - - Application - true - v141 - Unicode - Static - - - Application - false - v141 - true - Unicode - Static - - - Application - false - v141 - true - Unicode - Static - - - Application - false - v141 - true - Unicode - Static - - - Application - true - v141 - Unicode - Static - - - Application - true - v141 - Unicode - Static - - - Application - true - v141 - Unicode - Static - - - Application - true - v141 - Unicode - Static - - - Application - true - v141 - Unicode - Static - - - Application - true - v141 - Unicode - Static - - - Application - false - v141 - true - Unicode - Static - - - Application - false - v141 - true - Unicode - Static - - - Application - false - v141 - true - Unicode - Static - - - Application - false - v141 - true - Unicode - Static - - - Application - false - v141 - true - Unicode - Static - - - Application - false - v141 - true - Unicode - Static - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - - - true - false - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - true - false - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - true - false - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - true - false - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - true - false - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - true - false - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - - - true - false - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - true - false - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - true - false - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - - - true - false - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - true - false - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - true - false - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - true - false - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - true - false - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - true - false - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - - - true - false - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - true - false - - - false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public - - - true - false - - - - NotUsing - Level3 - MinSpace - false - false - true - HAVE_MAT_SHORT_NS;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - Guard - Default - true - false - false - false - false - false - false - Disabled - Size - true - Fast - false - - - - Console - true - true - true - false - true - true - Default - false - /merge:.rdata=.text - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - NotUsing - Level3 - MinSpace - false - false - true - HAVE_MAT_SHORT_NS;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - - Guard - Default - true - false - false - false - false - false - false - Disabled - Size - true - Fast - false - - - - Console - true - true - true - false - true - true - Default - false - /merge:.rdata=.text - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - - - - - NotUsing - Level3 - MinSpace - false - false - true - HAVE_MAT_SHORT_NS;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - - Guard - Default - true - Cdecl - false - false - false - false - false - false - Disabled - Size - true - Fast - false - - - - Console - true - true - true - wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) - false - true - true - Default - false - /merge:.rdata=.text - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - - - - - NotUsing - Level3 - MinSpace - false - false - true - HAVE_MAT_SHORT_NS;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - - Guard - Default - true - MultiThreaded - false - false - false - false - false - false - Disabled - Size - true - Fast - false - - false - - - Console - true - true - true - libcmt.lib;libvcruntime.lib;libucrt.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) - true - false - true - Default - false - /merge:.rdata=.text - false - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - - - - - NotUsing - Level3 - MinSpace - false - false - true - HAVE_MAT_SHORT_NS;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - - Guard - Default - true - Cdecl - false - false - false - false - false - false - Disabled - Size - true - Fast - false - - - - Console - true - true - true - wininet.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) - false - true - true - Default - false - /merge:.rdata=.text - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - - - - - NotUsing - Level3 - MinSpace - false - false - true - HAVE_MAT_SHORT_NS;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - - Guard - Default - true - MultiThreaded - false - false - false - false - false - false - Disabled - Size - true - Fast - false - - false - - - Console - true - true - true - libcmt.lib;libvcruntime.lib;libucrt.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) - true - false - true - Default - false - /merge:.rdata=.text - false - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - - - - - NotUsing - Level3 - MinSpace - true - HAVE_MAT_SHORT_NS;WIN32;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - Guard - ProgramDatabase - true - true - false - false - false - false - true - Default - false - false - false - Disabled - Size - false - Fast - false - - - - Console - true - false - true - true - true - true - Default - false - /merge:.rdata=.text - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - - - - - NotUsing - Level3 - MinSpace - true - HAVE_MAT_SHORT_NS;WIN32;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - - Guard - ProgramDatabase - true - true - false - false - false - false - true - Default - false - false - false - Disabled - Size - false - Fast - false - - - - Console - true - wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) - false - true - true - true - true - Default - false - /merge:.rdata=.text - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - - - - - NotUsing - Level3 - MinSpace - true - HAVE_MAT_SHORT_NS;WIN32;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - - Guard - ProgramDatabase - true - true - false - MultiThreadedDebug - false - false - false - true - Default - false - false - false - Disabled - Size - false - Fast - false - - - - Console - true - libucrtd.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) - false - true - true - true - true - Default - false - /merge:.rdata=.text - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - - - - - NotUsing - Level3 - MinSpace - true - HAVE_MAT_SHORT_NS;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - Guard - ProgramDatabase - true - false - false - false - true - Default - false - false - false - false - Disabled - Size - true - false - Fast - false - - - - Console - true - false - true - true - true - true - Default - false - /merge:.rdata=.text - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - - - - - NotUsing - Level3 - MinSpace - true - HAVE_MAT_SHORT_NS;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - - Guard - ProgramDatabase - true - false - false - false - true - Default - false - false - false - false - Disabled - Size - true - false - Fast - false - - - - Console - true - false - true - true - true - true - Default - false - /merge:.rdata=.text - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - - - - - NotUsing - Level3 - MinSpace - true - HAVE_MAT_SHORT_NS;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - - Guard - ProgramDatabase - true - Cdecl - false - false - false - true - Default - false - false - false - false - Disabled - Size - true - false - Fast - false - - - - Console - true - wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) - false - true - true - true - true - Default - false - /merge:.rdata=.text - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - - - - - NotUsing - Level3 - MinSpace - true - HAVE_MAT_SHORT_NS;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - - Guard - ProgramDatabase - true - MultiThreadedDebug - false - false - false - true - Default - false - false - false - false - Disabled - Size - true - false - Fast - false - - - - Console - true - libucrtd.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) - false - true - true - true - true - Default - false - /merge:.rdata=.text - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - - - - - NotUsing - Level3 - MinSpace - true - HAVE_MAT_SHORT_NS;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - - Guard - ProgramDatabase - true - false - false - false - true - Default - false - false - false - false - Disabled - Size - true - false - Fast - false - - - - Console - true - wininet.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) - false - true - true - true - true - Default - false - /merge:.rdata=.text - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - - - - - NotUsing - Level3 - MinSpace - true - HAVE_MAT_SHORT_NS;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - - Guard - ProgramDatabase - true - MultiThreadedDebug - false - false - false - true - Default - false - false - false - false - Disabled - Size - true - false - Fast - false - - - - Console - true - libucrtd.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) - false - true - true - true - true - Default - false - /merge:.rdata=.text - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - - - - - NotUsing - Level3 - MinSpace - false - false - true - HAVE_MAT_SHORT_NS;WIN32;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - Guard - Default - false - false - false - false - false - false - Disabled - Size - true - true - Fast - false - - - - Console - true - true - true - false - true - true - Default - false - /merge:.rdata=.text - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - - - - - NotUsing - Level3 - MinSpace - false - false - true - HAVE_MAT_SHORT_NS;WIN32;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - - Guard - Default - Cdecl - false - false - false - false - false - false - Disabled - Size - true - true - Fast - false - - - - Console - true - true - true - wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) - false - true - true - Default - false - /merge:.rdata=.text - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - - - - - NotUsing - Level3 - MinSpace - false - false - true - HAVE_MAT_SHORT_NS;WIN32;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) - false - - - Guard - Default - Cdecl - MultiThreaded - false - false - false - false - false - false - Disabled - Size - true - true - Fast - false - - false - - - Console - true - true - true - libcmt.lib;libvcruntime.lib;libucrt.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) - true - false - true - Default - false - /merge:.rdata=.text - false - false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - - - - - - - type nul >> main.cpp - - - - - - - - - - - - - - - - - - - - - - {1dc6b38a-b390-34ce-907f-4958807a3d43} - - - - - + + + + + Debug.static + ARM64 + + + Debug.static + Win32 + + + Debug.static + x64 + + + Debug.vs2015.MT-sqlite + ARM64 + + + Debug.vs2015.MT-sqlite + Win32 + + + Debug.vs2015.MT-sqlite + x64 + + + Debug + ARM64 + + + Debug + Win32 + + + Release.static + ARM64 + + + Release.static + Win32 + + + Release.static + x64 + + + Release.vs2015.MT-sqlite + ARM64 + + + Release.vs2015.MT-sqlite + Win32 + + + Release.vs2015.MT-sqlite + x64 + + + Release + ARM64 + + + Release + Win32 + + + Debug + x64 + + + Release + x64 + + + + 15.0 + {86AC752C-5687-4377-841E-943D9BEEF361} + Win32Proj + SampleCppMini + 10.0.17763.0 + true + + + + Application + true + v141 + Unicode + Static + + + Application + true + v141 + Unicode + Static + + + Application + true + v141 + Unicode + Static + + + Application + false + v141 + true + Unicode + Static + + + Application + false + v141 + true + Unicode + Static + + + Application + false + v141 + true + Unicode + Static + + + Application + true + v141 + Unicode + Static + + + Application + true + v141 + Unicode + Static + + + Application + true + v141 + Unicode + Static + + + Application + true + v141 + Unicode + Static + + + Application + true + v141 + Unicode + Static + + + Application + true + v141 + Unicode + Static + + + Application + false + v141 + true + Unicode + Static + + + Application + false + v141 + true + Unicode + Static + + + Application + false + v141 + true + Unicode + Static + + + Application + false + v141 + true + Unicode + Static + + + Application + false + v141 + true + Unicode + Static + + + Application + false + v141 + true + Unicode + Static + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + + + true + false + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + true + false + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + true + false + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + true + false + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + true + false + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + true + false + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + + + true + false + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + true + false + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + true + false + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + + + true + false + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + true + false + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + true + false + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + true + false + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + true + false + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + true + false + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + + + true + false + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + true + false + + + false + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + + + true + false + + + + NotUsing + Level3 + MinSpace + false + false + true + HAVE_MAT_SHORT_NS;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + Guard + Default + true + false + false + false + false + false + false + Disabled + Size + true + Fast + false + + MultiThreadedDLL + + + Console + true + true + true + false + true + true + Default + false + /merge:.rdata=.text + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + wininet.lib;Crypt32.lib; + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + NotUsing + Level3 + MinSpace + false + false + true + HAVE_MAT_SHORT_NS;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + + Guard + Default + true + false + false + false + false + false + false + Disabled + Size + true + Fast + false + + MultiThreadedDLL + + + Console + true + true + true + false + true + true + Default + false + /merge:.rdata=.text + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + wininet.lib;Crypt32.lib; + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + + + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + + + + + NotUsing + Level3 + MinSpace + false + false + true + HAVE_MAT_SHORT_NS;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + + Guard + Default + true + Cdecl + false + false + false + false + false + false + Disabled + Size + true + Fast + false + + + + Console + true + true + true + wininet.lib;Crypt32.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + false + true + true + Default + false + /merge:.rdata=.text + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + + + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + + + + + NotUsing + Level3 + MinSpace + false + false + true + HAVE_MAT_SHORT_NS;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + + Guard + Default + true + MultiThreaded + false + false + false + false + false + false + Disabled + Size + true + Fast + false + + false + + + Console + true + true + true + wininet.lib;Crypt32.lib;libcmt.lib;libvcruntime.lib;libucrt.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + true + false + true + Default + false + /merge:.rdata=.text + false + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + + + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + + + + + NotUsing + Level3 + MinSpace + false + false + true + HAVE_MAT_SHORT_NS;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + + Guard + Default + true + Cdecl + false + false + false + false + false + false + Disabled + Size + true + Fast + false + + + + Console + true + true + true + wininet.lib;Crypt32.lib;wininet.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) + false + true + true + Default + false + /merge:.rdata=.text + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + + + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + + + + + NotUsing + Level3 + MinSpace + false + false + true + HAVE_MAT_SHORT_NS;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + + Guard + Default + true + MultiThreaded + false + false + false + false + false + false + Disabled + Size + true + Fast + false + + false + + + Console + true + true + true + wininet.lib;Crypt32.lib;libcmt.lib;libvcruntime.lib;libucrt.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) + true + false + true + Default + false + /merge:.rdata=.text + false + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + + + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + + + + + NotUsing + Level3 + MinSpace + true + HAVE_MAT_SHORT_NS;WIN32;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + Guard + ProgramDatabase + true + true + false + false + false + false + true + Default + false + false + false + Disabled + Size + false + Fast + false + + MultiThreadedDebugDLL + + + Console + true + false + true + true + true + true + Default + false + /merge:.rdata=.text + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + wininet.lib;Crypt32.lib; + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + + + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + + + + + NotUsing + Level3 + MinSpace + true + HAVE_MAT_SHORT_NS;WIN32;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + + Guard + ProgramDatabase + true + true + false + false + false + false + true + Default + false + false + false + Disabled + Size + false + Fast + false + + + + Console + true + wininet.lib;Crypt32.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + false + true + true + true + true + Default + false + /merge:.rdata=.text + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + + + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + + + + + NotUsing + Level3 + MinSpace + true + HAVE_MAT_SHORT_NS;WIN32;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + + Guard + ProgramDatabase + true + true + false + MultiThreadedDebug + false + false + false + true + Default + false + false + false + Disabled + Size + false + Fast + false + + + + Console + true + wininet.lib;Crypt32.lib;libucrtd.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + false + true + true + true + true + Default + false + /merge:.rdata=.text + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + + + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + + + + + NotUsing + Level3 + MinSpace + true + HAVE_MAT_SHORT_NS;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + Guard + ProgramDatabase + true + false + false + false + true + Default + false + false + false + false + Disabled + Size + true + false + Fast + false + + MultiThreadedDebugDLL + + + Console + true + false + true + true + true + true + Default + false + /merge:.rdata=.text + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + wininet.lib;Crypt32.lib; + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + + + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + + + + + NotUsing + Level3 + MinSpace + true + HAVE_MAT_SHORT_NS;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + + Guard + ProgramDatabase + true + false + false + false + true + Default + false + false + false + false + Disabled + Size + true + false + Fast + false + + MultiThreadedDebugDLL + + + Console + true + false + true + true + true + true + Default + false + /merge:.rdata=.text + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + wininet.lib;Crypt32.lib; + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + + + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + + + + + NotUsing + Level3 + MinSpace + true + HAVE_MAT_SHORT_NS;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + + Guard + ProgramDatabase + true + Cdecl + false + false + false + true + Default + false + false + false + false + Disabled + Size + true + false + Fast + false + + + + Console + true + wininet.lib;Crypt32.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + false + true + true + true + true + Default + false + /merge:.rdata=.text + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + + + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + + + + + NotUsing + Level3 + MinSpace + true + HAVE_MAT_SHORT_NS;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + + Guard + ProgramDatabase + true + MultiThreadedDebug + false + false + false + true + Default + false + false + false + false + Disabled + Size + true + false + Fast + false + + + + Console + true + wininet.lib;Crypt32.lib;libucrtd.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + false + true + true + true + true + Default + false + /merge:.rdata=.text + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + + + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + + + + + NotUsing + Level3 + MinSpace + true + HAVE_MAT_SHORT_NS;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + + Guard + ProgramDatabase + true + false + false + false + true + Default + false + false + false + false + Disabled + Size + true + false + Fast + false + + + + Console + true + wininet.lib;Crypt32.lib;wininet.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) + false + true + true + true + true + Default + false + /merge:.rdata=.text + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + + + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + + + + + NotUsing + Level3 + MinSpace + true + HAVE_MAT_SHORT_NS;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + + Guard + ProgramDatabase + true + MultiThreadedDebug + false + false + false + true + Default + false + false + false + false + Disabled + Size + true + false + Fast + false + + + + Console + true + wininet.lib;Crypt32.lib;libucrtd.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) + false + true + true + true + true + Default + false + /merge:.rdata=.text + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + + + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + + + + + NotUsing + Level3 + MinSpace + false + false + true + HAVE_MAT_SHORT_NS;WIN32;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + Guard + Default + false + false + false + false + false + false + Disabled + Size + true + true + Fast + false + + MultiThreadedDLL + + + Console + true + true + true + false + true + true + Default + false + /merge:.rdata=.text + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + wininet.lib;Crypt32.lib; + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + + + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + + + + + NotUsing + Level3 + MinSpace + false + false + true + HAVE_MAT_SHORT_NS;WIN32;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + + Guard + Default + Cdecl + false + false + false + false + false + false + Disabled + Size + true + true + Fast + false + + + + Console + true + true + true + wininet.lib;Crypt32.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + false + true + true + Default + false + /merge:.rdata=.text + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + + + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + + + + + NotUsing + Level3 + MinSpace + false + false + true + HAVE_MAT_SHORT_NS;WIN32;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) + false + + + Guard + Default + Cdecl + MultiThreaded + false + false + false + false + false + false + Disabled + Size + true + true + Fast + false + + false + + + Console + true + true + true + wininet.lib;Crypt32.lib;libcmt.lib;libvcruntime.lib;libucrt.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + true + false + true + Default + false + /merge:.rdata=.text + false + false + API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 + + + $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) + + + Copy DLL to target dir + + + + + + + type nul >> main.cpp + + + + + + + + + + + + + + + + + + + + + + {1dc6b38a-b390-34ce-907f-4958807a3d43} + + + + + \ No newline at end of file diff --git a/lib/http/HttpClient_WinInet.cpp b/lib/http/HttpClient_WinInet.cpp index d5aba6311..90dbcef1c 100644 --- a/lib/http/HttpClient_WinInet.cpp +++ b/lib/http/HttpClient_WinInet.cpp @@ -3,7 +3,8 @@ #include "mat/config.h" #ifdef HAVE_MAT_DEFAULT_HTTP_CLIENT - +#pragma warning(push) +#pragma warning(disable:4189) /* Turn off Level 4: local variable is initialized but not referenced. dwError unused in Release without printing it. */ #include "HttpClient_WinInet.hpp" #include "utils/Utils.hpp" @@ -557,6 +558,6 @@ bool HttpClient_WinInet::IsMsRootCheckRequired() } } ARIASDK_NS_END - +#pragma warning(pop) #endif // HAVE_MAT_DEFAULT_HTTP_CLIENT // clang-format on diff --git a/lib/include/public/LogManagerBase.hpp b/lib/include/public/LogManagerBase.hpp index 67f9110b2..53fc5ab62 100644 --- a/lib/include/public/LogManagerBase.hpp +++ b/lib/include/public/LogManagerBase.hpp @@ -90,7 +90,7 @@ public ref class LogManagerLock { } #ifdef _MANAGED -#define LM_LOCKGUARD(macro_mutex) msclr::lock l(LogManagerLock::lock); +#define LM_LOCKGUARD(macro_mutex) msclr::lock TOKENPASTE2(__guard_, __LINE__)(LogManagerLock::lock); #else #define LM_LOCKGUARD(macro_mutex) \ std::lock_guard TOKENPASTE2(__guard_, __LINE__) (macro_mutex); diff --git a/lib/offline/OfflineStorageHandler.cpp b/lib/offline/OfflineStorageHandler.cpp index 5038fce7a..dfdc380d7 100644 --- a/lib/offline/OfflineStorageHandler.cpp +++ b/lib/offline/OfflineStorageHandler.cpp @@ -61,7 +61,7 @@ namespace ARIASDK_NS_BEGIN { if (!m_flushPending) return; } - LOG_INFO("Waiting for pending Flush (%p) to complete...", m_flushHandle.m_task); + LOG_INFO("Waiting for pending Flush (%p) to complete...", m_flushHandle.m_task.load()); m_flushComplete.wait(); } @@ -257,7 +257,7 @@ namespace ARIASDK_NS_BEGIN { m_flushPending = true; m_flushComplete.Reset(); m_flushHandle = PAL::scheduleTask(&m_taskDispatcher, 0, this, &OfflineStorageHandler::Flush); - LOG_INFO("Requested Flush (%p)", m_flushHandle.m_task); + LOG_INFO("Requested Flush (%p)", m_flushHandle.m_task.load()); } m_flushLock.unlock(); } diff --git a/lib/pal/TaskDispatcher.hpp b/lib/pal/TaskDispatcher.hpp index 391ac11ca..6b418b8b8 100644 --- a/lib/pal/TaskDispatcher.hpp +++ b/lib/pal/TaskDispatcher.hpp @@ -11,6 +11,7 @@ #include #include #include +#include #include "ITaskDispatcher.hpp" #include "Version.hpp" @@ -58,7 +59,7 @@ namespace PAL_NS_BEGIN { class DeferredCallbackHandle { public: - MAT::Task* m_task; + std::atomic m_task; MAT::ITaskDispatcher* m_taskDispatcher; DeferredCallbackHandle(MAT::Task* task, MAT::ITaskDispatcher* taskDispatcher) : @@ -66,16 +67,22 @@ namespace PAL_NS_BEGIN { m_taskDispatcher(taskDispatcher) { }; DeferredCallbackHandle() : m_task(nullptr), m_taskDispatcher(nullptr) {}; DeferredCallbackHandle(const DeferredCallbackHandle& h) : - m_task(h.m_task), + m_task(h.m_task.load()), m_taskDispatcher(h.m_taskDispatcher) { }; + DeferredCallbackHandle& operator=(DeferredCallbackHandle other) + { + m_task = other.m_task.load(); + m_taskDispatcher = other.m_taskDispatcher; + return *this; + } + bool Cancel(uint64_t waitTime = 0) { - if (m_task) + MAT::Task* m_current_task = m_task.exchange(nullptr); + if (m_current_task) { - bool result = (m_taskDispatcher != nullptr) && (m_taskDispatcher->Cancel(m_task, waitTime)); - m_task = nullptr; - m_taskDispatcher = nullptr; + bool result = (m_taskDispatcher != nullptr) && (m_taskDispatcher->Cancel(m_current_task, waitTime)); return result; } return false; diff --git a/lib/pal/WorkerThread.cpp b/lib/pal/WorkerThread.cpp index cdda2f50f..99d473869 100644 --- a/lib/pal/WorkerThread.cpp +++ b/lib/pal/WorkerThread.cpp @@ -42,6 +42,7 @@ namespace PAL_NS_BEGIN { WorkerThread() { + m_itemInProgress = nullptr; m_hThread = std::thread(WorkerThread::threadFunc, static_cast(this)); LOG_INFO("Started new thread %u", m_hThread.get_id()); } @@ -95,21 +96,34 @@ namespace PAL_NS_BEGIN { m_event.post(); } - // TODO: [maxgolov] - method has to be refactored to allow cancellation by Task tid. - // Otherwise we may run into a situation where we wait on a 'wrong task' completion. - // In case if the task we were supposed to be waiting on is done running and destroyed, - // but while we idle-wait for TASK_CANCEL_WAIT_MS - a new task obj got allocated on - // heap with the same exact raw ptr. The chance of that collision is slim and the only - // side-effect is we'd idle-wait slightly longer until the new task is done. + // Cancel a task or wait for task completion for up to waitTime ms: // - // It would also make sense to return the following cancellation status: - // - task not found - // - task found and cancelled - // - task found, but not cancelled - ran to completion. - // - task found, but not cancelled - still running. + // - acquire the m_lock to prevent a new task from getting scheduled. + // This may block the scheduling of a new task in queue for up to + // TASK_CANCEL_WAIT_MS=50 ms in case if the task being canceled + // is the one being executed right now. + // + // - if currently executing task is the one we are trying to cancel, + // then verify for recursion: if the current thread is the same + // we're waiting on, prevent the recursion (we can't cancel our own + // thread task). If it's different thread, then idle-poll-wait for + // task completion for up to waitTime ms. m_itemInProgress is nullptr + // once the item is done executing. Method may fail and return if + // waitTime given was insufficient to wait for completion. + // + // - if task being cancelled is not executing yet, then erase it from + // timer queue without any wait. + // + // TODO: current callers of this API do not check the status code. + // Refactor this code to return the following cancellation status: + // - TASK_NOTFOUND - task not found + // - TASK_CANCELLED - task found and cancelled without execution + // - TASK_COMPLETED - task found and ran to completion + // - TASK_RUNNING - task is still running (insufficient waitTime) // bool Cancel(MAT::Task* item, uint64_t waitTime) override { + LOCKGUARD(m_lock); if (item == nullptr) { return false; @@ -120,7 +134,6 @@ namespace PAL_NS_BEGIN { /* Can't recursively wait on completion of our own thread */ if (m_hThread.get_id() != std::this_thread::get_id()) { - LOCKGUARD(m_lock); while ((waitTime > TASK_CANCEL_WAIT_MS) && (m_itemInProgress == item)) { PAL::sleep(TASK_CANCEL_WAIT_MS); @@ -135,7 +148,6 @@ namespace PAL_NS_BEGIN { } { - LOCKGUARD(m_lock); auto it = std::find(m_timerQueue.begin(), m_timerQueue.end(), item); if (it != m_timerQueue.end()) { // Still in the queue @@ -199,6 +211,10 @@ namespace PAL_NS_BEGIN { item = std::unique_ptr(self->m_queue.front()); self->m_queue.pop_front(); } + + if (item) { + self->m_itemInProgress = item.get(); + } } if (!item) { @@ -209,11 +225,11 @@ namespace PAL_NS_BEGIN { if (item->Type == MAT::Task::Shutdown) { item.reset(); + self->m_itemInProgress = nullptr; break; } LOG_TRACE("%10llu Execute item=%p type=%s\n", wakeupCount, item.get(), item.get()->TypeName.c_str() ); - self->m_itemInProgress = item.get(); (*item)(); self->m_itemInProgress = nullptr; diff --git a/lib/tpm/TransmissionPolicyManager.cpp b/lib/tpm/TransmissionPolicyManager.cpp index 6a96c0cf7..01136e2cc 100644 --- a/lib/tpm/TransmissionPolicyManager.cpp +++ b/lib/tpm/TransmissionPolicyManager.cpp @@ -228,10 +228,10 @@ namespace ARIASDK_NS_BEGIN { LOCKGUARD(m_scheduledUploadMutex); // Prevent execution of all upload tasks m_scheduledUploadAborted = true; + // Make sure we wait for completion of the upload scheduling task that may be running + cancelUploadTask(); } - // Make sure we wait for completion on the upload scheduling task that may be running - cancelUploadTask(); // Make sure we wait for all active upload callbacks to finish while (uploadCount() > 0) { From 6711d96ab17a7e218213e936e4b24842e5e915de Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Mon, 9 Mar 2020 14:45:13 -0700 Subject: [PATCH 09/11] Address code review concern --- lib/tpm/TransmissionPolicyManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tpm/TransmissionPolicyManager.cpp b/lib/tpm/TransmissionPolicyManager.cpp index 01136e2cc..79a82c1b4 100644 --- a/lib/tpm/TransmissionPolicyManager.cpp +++ b/lib/tpm/TransmissionPolicyManager.cpp @@ -139,12 +139,12 @@ namespace ARIASDK_NS_BEGIN { void TransmissionPolicyManager::uploadAsync(EventLatency latency) { - m_isUploadScheduled = false; // Allow to schedule another uploadAsync m_runningLatency = latency; m_scheduledUploadTime = std::numeric_limits::max(); { LOCKGUARD(m_scheduledUploadMutex); + m_isUploadScheduled = false; // Allow to schedule another uploadAsync if ((m_isPaused) || (m_scheduledUploadAborted)) { LOG_TRACE("Paused or upload aborted: cancel pending upload task."); From 0dd5a728fa6e9eece542b6faf38d32044a950963 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Mon, 9 Mar 2020 19:51:40 -0700 Subject: [PATCH 10/11] Update lib/http/HttpClient_WinInet.cpp Co-Authored-By: Reiley Yang --- lib/http/HttpClient_WinInet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/http/HttpClient_WinInet.cpp b/lib/http/HttpClient_WinInet.cpp index 90dbcef1c..22a9fcd4d 100644 --- a/lib/http/HttpClient_WinInet.cpp +++ b/lib/http/HttpClient_WinInet.cpp @@ -131,7 +131,7 @@ class WinInetRequestWrapper } // Asynchronously send HTTP request and invoke response callback. - // Onwership semantics: send(...) method self-destroys *this* upon + // Ownership semantics: send(...) method self-destroys *this* upon // receiving WinInet callback. There must be absolutely no methods // that attempt to use the object after triggering send on it. // Send operation on request may be issued no more than once. From 1d2fb2937a6afbefdcd1ee656461aa9656704303 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Mon, 9 Mar 2020 19:51:52 -0700 Subject: [PATCH 11/11] Update lib/http/HttpClient_WinInet.cpp Co-Authored-By: Reiley Yang --- lib/http/HttpClient_WinInet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/http/HttpClient_WinInet.cpp b/lib/http/HttpClient_WinInet.cpp index 22a9fcd4d..b493e1e76 100644 --- a/lib/http/HttpClient_WinInet.cpp +++ b/lib/http/HttpClient_WinInet.cpp @@ -261,7 +261,7 @@ class WinInetRequestWrapper onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); return; } - // Async request has been queued in WinInet thread pool + // Async request has been queued in WinInet thread pool } static void CALLBACK winInetCallback(HINTERNET hInternet, DWORD_PTR dwContext, DWORD dwInternetStatus, LPVOID lpvStatusInformation, DWORD dwStatusInformationLength)