[ETW Exporter] - ETW provider handle cleanup (#1322)

lalitb · web-flow · commit d72dcb69b07f · 2022-04-11T00:45:36.000-07:00
diff --git a/exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h b/exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h
@@ -203,7 +203,8 @@ class ETWProvider
   {
     std::lock_guard<std::mutex> lock(m_providerMapLock);
 
-    auto m  = providers();
+    // use reference to provider list, NOT it' copy.
+    auto &m = providers();
     auto it = m.begin();
     while (it != m.end())
     {
@@ -228,7 +229,10 @@ class ETWProvider
           }
 
           it->second.providerHandle = INVALID_HANDLE;
-          m.erase(it);
+          if (result == STATUS_OK)
+          {
+            m.erase(it);
+          }
         }
         return result;
       }
diff --git a/exporters/etw/test/etw_provider_test.cc b/exporters/etw/test/etw_provider_test.cc
@@ -18,6 +18,7 @@ TEST(ETWProvider, ProviderIsRegisteredSuccessfully)
 
   bool registered = etw.is_registered(providerName);
   ASSERT_TRUE(registered);
+  etw.close(handle);
 }
 
 TEST(ETWProvider, ProviderIsNotRegisteredSuccessfully)
@@ -46,17 +47,18 @@ TEST(ETWProvider, CheckOpenGUIDDataSuccessfully)
   auto guidStrName = uuid_name.to_string();
 
   ASSERT_STREQ(guidStrHandle.c_str(), guidStrName.c_str());
+  etw.close(handle);
 }
 
 TEST(ETWProvider, CheckCloseSuccess)
 {
   std::string providerName = "OpenTelemetry-ETW-Provider";
 
   static ETWProvider etw;
-  auto handle = etw.open(providerName.c_str());
-
+  auto handle = etw.open(providerName.c_str(), ETWProvider::EventFormat::ETW_MANIFEST);
   auto result = etw.close(handle);
   ASSERT_NE(result, etw.STATUS_ERROR);
+  ASSERT_FALSE(etw.is_registered(providerName));
 }
 
 #endif

Original file line number	Diff line number	Diff line change
`@@ -203,7 +203,8 @@ class ETWProvider`
`203`	`203`	`{`
`204`	`204`	`std::lock_guard<std::mutex> lock(m_providerMapLock);`
`205`	`205`
`206`		`- auto m = providers();`
	`206`	`+ // use reference to provider list, NOT it' copy.`
	`207`	`+ auto &m = providers();`
`207`	`208`	`auto it = m.begin();`
`208`	`209`	`while (it != m.end())`
`209`	`210`	`{`
`@@ -228,7 +229,10 @@ class ETWProvider`
`228`	`229`	`}`
`229`	`230`
`230`	`231`	`it->second.providerHandle = INVALID_HANDLE;`
`231`		`- m.erase(it);`
	`232`	`+ if (result == STATUS_OK)`
	`233`	`+ {`
	`234`	`+ m.erase(it);`
	`235`	`+ }`
`232`	`236`	`}`
`233`	`237`	`return result;`
`234`	`238`	`}`
Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,7 @@ TEST(ETWProvider, ProviderIsRegisteredSuccessfully)`
`18`	`18`
`19`	`19`	`bool registered = etw.is_registered(providerName);`
`20`	`20`	`ASSERT_TRUE(registered);`
	`21`	`+ etw.close(handle);`
`21`	`22`	`}`
`22`	`23`
`23`	`24`	`TEST(ETWProvider, ProviderIsNotRegisteredSuccessfully)`
`@@ -46,17 +47,18 @@ TEST(ETWProvider, CheckOpenGUIDDataSuccessfully)`
`46`	`47`	`auto guidStrName = uuid_name.to_string();`
`47`	`48`
`48`	`49`	`ASSERT_STREQ(guidStrHandle.c_str(), guidStrName.c_str());`
	`50`	`+ etw.close(handle);`
`49`	`51`	`}`
`50`	`52`
`51`	`53`	`TEST(ETWProvider, CheckCloseSuccess)`
`52`	`54`	`{`
`53`	`55`	`std::string providerName = "OpenTelemetry-ETW-Provider";`
`54`	`56`
`55`	`57`	`static ETWProvider etw;`
`56`		`- auto handle = etw.open(providerName.c_str());`
`57`		`-`
	`58`	`+ auto handle = etw.open(providerName.c_str(), ETWProvider::EventFormat::ETW_MANIFEST);`
`58`	`59`	`auto result = etw.close(handle);`
`59`	`60`	`ASSERT_NE(result, etw.STATUS_ERROR);`
	`61`	`+ ASSERT_FALSE(etw.is_registered(providerName));`
`60`	`62`	`}`
`61`	`63`
`62`	`64`	`#endif`