Skip to content

Remove global C++ objects #1054

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Remove global C++ objects #1054

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6fd3d9b
Remove global C++ objects
tkukielk Sep 12, 2022
c475d6b
Fixing uint tests build
tkukielk Sep 13, 2022
0945e3b
Bring back public methods and change debugLogStream to shared_ptr
tkukielk Sep 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 7 additions & 12 deletions lib/api/LogManagerFactory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,16 @@

namespace MAT_NS_BEGIN
{
// This mutex has to be recursive because we allow both
// Destroy and destrutor to lock it. destructor could be
// called directly, and Destroy calls destructor.
std::recursive_mutex ILogManagerInternal::managers_lock;
std::set<ILogManager*> ILogManagerInternal::managers;

/// <summary>
/// Creates an instance of ILogManager using specified configuration.
/// </summary>
/// <param name="configuration">The configuration.</param>
/// <returns>ILogManager instance</returns>
ILogManager* LogManagerFactory::Create(ILogConfiguration& configuration)
{
LOCKGUARD(ILogManagerInternal::managers_lock);
LOCKGUARD(ILogManagerInternal::GetManagersLock());
auto logManager = new LogManagerImpl(configuration);
ILogManagerInternal::managers.emplace(logManager);
ILogManagerInternal::GetManagers().emplace(logManager);
return logManager;
}

Expand All @@ -50,11 +44,12 @@ namespace MAT_NS_BEGIN
return STATUS_EFAIL;
}

LOCKGUARD(ILogManagerInternal::managers_lock);
auto it = ILogManagerInternal::managers.find(instance);
if (it != std::end(ILogManagerInternal::managers))
LOCKGUARD(ILogManagerInternal::GetManagersLock());
std::set<ILogManager*>& managers = ILogManagerInternal::GetManagers();
auto it = managers.find(instance);
if (it != std::end(managers))
{
ILogManagerInternal::managers.erase(it);
managers.erase(it);
delete instance;
return STATUS_SUCCESS;
}
Expand Down
26 changes: 22 additions & 4 deletions lib/api/LogManagerImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,24 @@

namespace MAT_NS_BEGIN
{
// This mutex has to be recursive because we allow both
// Destroy and destrutor to lock it. destructor could be
// called directly, and Destroy calls destructor.

// static
std::recursive_mutex& ILogManagerInternal::GetManagersLock()
{
static std::recursive_mutex managers_lock;
return managers_lock;
};

// static
std::set<ILogManager*>& ILogManagerInternal::GetManagers() noexcept
{
static std::set<ILogManager*> managers;
return managers;
}

void DeadLoggers::AddMap(LoggerMap&& source)
{
std::lock_guard<std::mutex> lock(m_deadLoggersMutex);
Expand All @@ -94,8 +112,8 @@ namespace MAT_NS_BEGIN

bool ILogManager::DispatchEventBroadcast(DebugEvent evt)
{
// LOCKGUARD(ILogManagerInternal::managers_lock);
for (auto instance : ILogManagerInternal::managers)
// LOCKGUARD(ILogManagerInternal::GetManagersLock());
for (auto instance : ILogManagerInternal::GetManagers())
{
instance->DispatchEvent(evt);
}
Expand Down Expand Up @@ -353,8 +371,8 @@ namespace MAT_NS_BEGIN
LogManagerImpl::~LogManagerImpl() noexcept
{
FlushAndTeardown();
LOCKGUARD(ILogManagerInternal::managers_lock);
ILogManagerInternal::managers.erase(this);
LOCKGUARD(ILogManagerInternal::GetManagersLock());
ILogManagerInternal::GetManagers().erase(this);
}

size_t LogManagerImpl::GetDeadLoggerCount()
Expand Down
4 changes: 2 additions & 2 deletions lib/api/LogManagerImpl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ namespace MAT_NS_BEGIN
class ILogManagerInternal : public ILogManager
{
public:
static std::recursive_mutex managers_lock;
static std::set<ILogManager*> managers;
static std::recursive_mutex& GetManagersLock();
static std::set<ILogManager*>& GetManagers() noexcept;

/// <summary>
/// Optional decorator runs on event before passing it to sendEvent
Expand Down
134 changes: 70 additions & 64 deletions lib/config/RuntimeConfig_Default.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,68 +10,6 @@

namespace MAT_NS_BEGIN
{
static ILogConfiguration defaultRuntimeConfig{
{CFG_INT_TRACE_LEVEL_MIN, ACTTraceLevel::ACTTraceLevel_Error},
{CFG_INT_SDK_MODE, SdkModeTypes::SdkModeTypes_CS},
{CFG_BOOL_ENABLE_ANALYTICS, false},
{CFG_INT_CACHE_FILE_SIZE, 3145728},
{CFG_INT_RAM_QUEUE_SIZE, 524288},
{CFG_BOOL_ENABLE_MULTITENANT, true},
{CFG_BOOL_ENABLE_DB_DROP_IF_FULL, false},
{CFG_INT_MAX_TEARDOWN_TIME, 1},
{CFG_INT_MAX_PENDING_REQ, 4},
{CFG_INT_RAM_QUEUE_BUFFERS, 3},
{CFG_INT_TRACE_LEVEL_MASK, 0},
{CFG_BOOL_ENABLE_TRACE, true},
{CFG_STR_COLLECTOR_URL, COLLECTOR_URL_PROD},
{CFG_INT_STORAGE_FULL_PCT, 75},
{CFG_INT_STORAGE_FULL_CHECK_TIME, 5000},
{CFG_INT_RAMCACHE_FULL_PCT, 75},
{CFG_BOOL_ENABLE_NET_DETECT, true},
{CFG_BOOL_SESSION_RESET_ENABLED, false},
{CFG_MAP_METASTATS_CONFIG,
{/* Parameter that allows to split stats events by tenant */
{"split", false},
{"interval", 1800},
{"tokenProd", STATS_TOKEN_PROD},
{"tokenInt", STATS_TOKEN_INT}}},
{"utc",
{
#ifdef HAVE_MAT_UTC
{"providerGroupId", "780dddc8-18a1-5781-895a-a690464fa89c"},
{CFG_BOOL_UTC_ENABLED, true},
{CFG_BOOL_UTC_ACTIVE, false},
{CFG_BOOL_UTC_LARGE_PAYLOADS, false}
#else
{CFG_BOOL_UTC_ENABLED, false}
#endif
}},
{CFG_MAP_HTTP,
{
#ifdef HAVE_MAT_ZLIB
{CFG_BOOL_HTTP_COMPRESSION, true}
#else
{CFG_BOOL_HTTP_COMPRESSION, false}
#endif
,
{"contentEncoding", "deflate"},
/* Optional parameter to require Microsoft Root CA */
{CFG_BOOL_HTTP_MS_ROOT_CHECK, false}}},
{CFG_MAP_TPM,
{
{CFG_INT_TPM_MAX_BLOB_BYTES, 2097152},
{CFG_INT_TPM_MAX_RETRY, 5},
{CFG_BOOL_TPM_CLOCK_SKEW_ENABLED, true},
{CFG_STR_TPM_BACKOFF, "E,3000,300000,2,1"},
}},
{CFG_MAP_COMPAT,
{
{CFG_BOOL_COMPAT_DOTS, true}, // false: v1 backwards-compat: event.SetType("My.Custom.Type") => custom.my_custom_type
{CFG_STR_COMPAT_PREFIX, EVENTRECORD_TYPE_CUSTOM_EVENT} // custom type prefix for Interchange / Geneva / Cosmos flow
}},
{"sample",
{{"rate", 0}}}};

/// <summary>
/// This class overlays a custom configuration provided by the customer
/// on top of default configuration above (defaultRuntimeConfig)
Expand All @@ -81,12 +19,80 @@ namespace MAT_NS_BEGIN
{
protected:
ILogConfiguration& config;


private:
static ILogConfiguration& GetDefaultRuntimeConfig()
{
static ILogConfiguration defaultRuntimeConfig {
{CFG_INT_TRACE_LEVEL_MIN, ACTTraceLevel::ACTTraceLevel_Error},
{CFG_INT_SDK_MODE, SdkModeTypes::SdkModeTypes_CS},
{CFG_BOOL_ENABLE_ANALYTICS, false},
{CFG_INT_CACHE_FILE_SIZE, 3145728},
{CFG_INT_RAM_QUEUE_SIZE, 524288},
{CFG_BOOL_ENABLE_MULTITENANT, true},
{CFG_BOOL_ENABLE_DB_DROP_IF_FULL, false},
{CFG_INT_MAX_TEARDOWN_TIME, 1},
{CFG_INT_MAX_PENDING_REQ, 4},
{CFG_INT_RAM_QUEUE_BUFFERS, 3},
{CFG_INT_TRACE_LEVEL_MASK, 0},
{CFG_BOOL_ENABLE_TRACE, true},
{CFG_STR_COLLECTOR_URL, COLLECTOR_URL_PROD},
{CFG_INT_STORAGE_FULL_PCT, 75},
{CFG_INT_STORAGE_FULL_CHECK_TIME, 5000},
{CFG_INT_RAMCACHE_FULL_PCT, 75},
{CFG_BOOL_ENABLE_NET_DETECT, true},
{CFG_BOOL_SESSION_RESET_ENABLED, false},
{CFG_MAP_METASTATS_CONFIG,
{/* Parameter that allows to split stats events by tenant */
{"split", false},
{"interval", 1800},
{"tokenProd", STATS_TOKEN_PROD},
{"tokenInt", STATS_TOKEN_INT}}},
{"utc",
{
#ifdef HAVE_MAT_UTC
{"providerGroupId", "780dddc8-18a1-5781-895a-a690464fa89c"},
{CFG_BOOL_UTC_ENABLED, true},
{CFG_BOOL_UTC_ACTIVE, false},
{CFG_BOOL_UTC_LARGE_PAYLOADS, false}
#else
{CFG_BOOL_UTC_ENABLED, false}
#endif
}},
{CFG_MAP_HTTP,
{
#ifdef HAVE_MAT_ZLIB
{CFG_BOOL_HTTP_COMPRESSION, true}
#else
{CFG_BOOL_HTTP_COMPRESSION, false}
#endif
,
{"contentEncoding", "deflate"},
/* Optional parameter to require Microsoft Root CA */
{CFG_BOOL_HTTP_MS_ROOT_CHECK, false}}},
{CFG_MAP_TPM,
{
{CFG_INT_TPM_MAX_BLOB_BYTES, 2097152},
{CFG_INT_TPM_MAX_RETRY, 5},
{CFG_BOOL_TPM_CLOCK_SKEW_ENABLED, true},
{CFG_STR_TPM_BACKOFF, "E,3000,300000,2,1"},
}},
{CFG_MAP_COMPAT,
{
{CFG_BOOL_COMPAT_DOTS, true}, // false: v1 backwards-compat: event.SetType("My.Custom.Type") => custom.my_custom_type
{CFG_STR_COMPAT_PREFIX, EVENTRECORD_TYPE_CUSTOM_EVENT} // custom type prefix for Interchange / Geneva / Cosmos flow
}},
{"sample",
{{"rate", 0}}}};

return defaultRuntimeConfig;
}

public:
RuntimeConfig_Default(ILogConfiguration& customConfig) :
config(customConfig)
{
Variant::merge_map(*customConfig, *defaultRuntimeConfig);
Variant::merge_map(*customConfig, *GetDefaultRuntimeConfig());
};

virtual ~RuntimeConfig_Default()
Expand Down
44 changes: 22 additions & 22 deletions lib/include/public/TransmitProfiles.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,6 @@ namespace MAT_NS_BEGIN
class TransmitProfiles {

protected:
/// <summary>
/// A map that contains all transmit profiles.
/// </summary>
static std::map<std::string, TransmitProfileRules> profiles;

/// <summary>
/// A string that contains the name of the currently active transmit profile.
/// </summary>
static std::string currProfileName;

/// <summary>
/// The size of the currently active transmit profile rule.
Expand All @@ -161,12 +152,33 @@ namespace MAT_NS_BEGIN
/// </summary>
static bool isTimerUpdated;

/// <summary>
/// A map that contains all transmit profiles.
/// </summary>
static std::map<std::string, TransmitProfileRules>& GetProfiles() noexcept;

/// <summary>
/// A string that contains the name of the currently active transmit profile.
/// </summary>
static std::string& GetCurrProfileName() noexcept;

static void UpdateProfiles(const std::vector<TransmitProfileRules>& newProfiles) noexcept;

static void EnsureDefaultProfiles() noexcept;

public:
/// <summary>
/// Removes custom profiles.
/// This method is called from parse only, and does not require the lock.
/// <b>Note:</b> This function is not thread safe.
/// </summary>
static void removeCustomProfiles();

/// <summary>
/// A timer update event handler.
/// </summary>
static void onTimersUpdated();

public:

/// <summary>
/// The TransmitProfiles default constructor.
Expand All @@ -183,13 +195,6 @@ namespace MAT_NS_BEGIN
/// </summary>
static void dump();

/// <summary>
/// Removes custom profiles.
/// This method is called from parse only, and does not require the lock.
/// <b>Note:</b> This function is not thread safe.
/// </summary>
static void removeCustomProfiles();

/// <summary>
/// Parses transmit profiles from JSON.
/// </summary>
Expand Down Expand Up @@ -249,11 +254,6 @@ namespace MAT_NS_BEGIN
/// <param name="powState">A reference to an instance of a MAT::PowerSource enumeration.</param>
static void getDeviceState(NetworkCost &netCost, PowerSource &powState);

/// <summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these methods being moved to Protected? These are public methods on a public header and are breaking changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I followed the comments on some of these methods and they looked like private and used only internally - especially the removeCustomProfiles(), which is not thread safe, used without a guarding lock and comments indicate it should not be exposed. But I can restore them to public so we don't restrict the visibility with this change

/// A timer update event handler.
/// </summary>
static void onTimersUpdated();

/// <summary>
/// Determines whether a timer should be updated.
/// </summary>
Expand Down
Loading