LGTM from niklase. It wasn't possible to create multiple instances of cpu windows. Additionally, cpu windows set process wide security. Additionally, the memory was never reclaimed because of an uninitialized member of the cpu windows class. All three issues should be fixed.
Review URL: http://webrtc-codereview.appspot.com/24006
git-svn-id: http://webrtc.googlecode.com/svn/trunk@15 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/system_wrappers/source/cpu_windows.cc b/system_wrappers/source/cpu_windows.cc
index 60923ac..22146bf 100644
--- a/system_wrappers/source/cpu_windows.cc
+++ b/system_wrappers/source/cpu_windows.cc
@@ -26,7 +26,7 @@
namespace webrtc {
WebRtc_Word32 CpuWindows::CpuUsage()
{
- if (!initialized_)
+ if (!has_initialized_)
{
return -1;
}
@@ -37,7 +37,7 @@
WebRtc_Word32 CpuWindows::CpuUsageMultiCore(WebRtc_UWord32& num_cores,
WebRtc_UWord32*& cpu_usage)
{
- if (!initialized_)
+ if (!has_initialized_)
{
return -1;
}
@@ -49,8 +49,9 @@
CpuWindows::CpuWindows()
: cpu_polling_thread(NULL),
initialize_(true),
- initialized_(false),
+ has_initialized_(false),
terminate_(false),
+ has_terminated_(false),
cpu_usage_(NULL),
wbem_enum_access_(NULL),
number_of_objects_(0),
@@ -59,6 +60,7 @@
timestamp_sys_100_ns_handle_(0),
previous_100ns_timestamp_(NULL),
wbem_service_(NULL),
+ wbem_service_proxy_(NULL),
wbem_refresher_(NULL),
wbem_enum_(NULL)
{
@@ -147,24 +149,24 @@
init_cond_->SleepCS(*init_crit_);
}
}
- if (!initialized_)
+ if (!has_initialized_)
{
cpu_polling_thread->Stop();
return false;
}
- return initialized_;
+ return has_initialized_;
}
bool CpuWindows::StopPollingCpu()
{
- if (!initialized_)
+ if (!has_initialized_)
{
return false;
}
CriticalSectionScoped cs(*terminate_crit_);
terminate_ = true;
sleep_event->Set();
- while (!terminated_)
+ while (!has_terminated_)
{
terminate_cond_->SleepCS(*terminate_crit_);
}
@@ -187,7 +189,6 @@
{
const bool success = Terminate();
assert(success);
- terminated_ = true;
terminate_cond_->WakeAll();
return false;
}
@@ -199,9 +200,9 @@
initialize_ = false;
const bool success = Initialize();
init_cond_->WakeAll();
- if (!success || !initialized_)
+ if (!success || !has_initialized_)
{
- initialized_ = false;
+ has_initialized_ = false;
terminate_ = true;
return false;
}
@@ -269,9 +270,31 @@
{
return false;
}
+
+ // Create a proxy to the IWbemServices so that a local authentication
+ // can be set up (this is needed to be able to successfully call
+ // IWbemConfigureRefresher::AddEnum). Setting authentication with
+ // CoInitializeSecurity is process-wide (which is too intrusive).
+ hr = CoCopyProxy(static_cast<IUnknown*> (wbem_service_),
+ reinterpret_cast<IUnknown**> (&wbem_service_proxy_));
+ if(FAILED(hr))
+ {
+ return false;
+ }
+ // Set local authentication.
+ // RPC_C_AUTHN_WINNT means using NTLM instead of Kerberos which is default.
+ hr = CoSetProxyBlanket(static_cast<IUnknown*> (wbem_service_proxy_),
+ RPC_C_AUTHN_WINNT, RPC_C_AUTHZ_NONE, NULL,
+ RPC_C_AUTHN_LEVEL_DEFAULT,
+ RPC_C_IMP_LEVEL_IMPERSONATE, NULL, EOAC_NONE);
+ if(FAILED(hr))
+ {
+ return false;
+ }
+
// Don't care about the particular id for the enum.
long enum_id = 0;
- hr = wbem_refresher_config->AddEnum(wbem_service_,
+ hr = wbem_refresher_config->AddEnum(wbem_service_proxy_,
L"Win32_PerfRawData_PerfOS_Processor",
0, NULL, &wbem_enum_, &enum_id);
wbem_refresher_config->Release();
@@ -359,8 +382,6 @@
{
return false;
}
- hr = CoInitializeSecurity(NULL, -1, NULL, NULL, RPC_C_AUTHN_LEVEL_NONE,
- RPC_C_IMP_LEVEL_IMPERSONATE, NULL, EOAC_NONE, 0);
if (FAILED(hr))
{
return false;
@@ -378,12 +399,16 @@
{
return false;
}
- initialized_ = true;
+ has_initialized_ = true;
return true;
}
bool CpuWindows::Terminate()
{
+ if (has_terminated_)
+ {
+ return false;
+ }
// Reverse order of Initialize().
// Some compilers complain about deleting NULL though it's well defined
if (previous_100ns_timestamp_ != NULL)
@@ -417,12 +442,17 @@
{
wbem_enum_->Release();
wbem_enum_ = NULL;
- }
+ }
if (wbem_refresher_ != NULL)
{
wbem_refresher_->Release();
wbem_refresher_ = NULL;
}
+ if (wbem_service_proxy_ != NULL)
+ {
+ wbem_service_proxy_->Release();
+ wbem_service_proxy_ = NULL;
+ }
if (wbem_service_ != NULL)
{
wbem_service_->Release();
@@ -431,7 +461,8 @@
// CoUninitialized should be called once for every CoInitializeEx.
// Regardless if it failed or not.
CoUninitialize();
- return false;
+ has_terminated_ = true;
+ return true;
}
bool CpuWindows::UpdateCpuUsage()
diff --git a/system_wrappers/source/cpu_windows.h b/system_wrappers/source/cpu_windows.h
index c06b4ad..34f6181 100644
--- a/system_wrappers/source/cpu_windows.h
+++ b/system_wrappers/source/cpu_windows.h
@@ -62,12 +62,12 @@
ThreadWrapper* cpu_polling_thread;
bool initialize_;
- bool initialized_;
+ bool has_initialized_;
CriticalSectionWrapper* init_crit_;
ConditionVariableWrapper* init_cond_;
bool terminate_;
- bool terminated_;
+ bool has_terminated_;
CriticalSectionWrapper* terminate_crit_;
ConditionVariableWrapper* terminate_cond_;
@@ -92,6 +92,7 @@
unsigned __int64* previous_100ns_timestamp_;
IWbemServices* wbem_service_;
+ IWbemServices* wbem_service_proxy_;
IWbemRefresher* wbem_refresher_;