From: Sean Gillespie <sean@swgillespie.me>
Date: Sat, 22 Apr 2017 11:39:09 +0000 (-0700)
Subject: Remove destructor from GCEvent and instead rely on the OS to clean up (dotnet/coreclr... 
X-Git-Tag: submit/tizen/20210909.063632~11030^2~7157
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=0640758b7d56b214ee379f99673c506dd39fd230;p=platform%2Fupstream%2Fdotnet%2Fruntime.git

Remove destructor from GCEvent and instead rely on the OS to clean up (dotnet/coreclr#11132)

* Remove destructor from GCEvent and instead rely on the OS to clean up

* Add a comment justifying the lack of destructor

* wording: many -> all


Commit migrated from https://github.com/dotnet/coreclr/commit/02fa8ca0932d02ce369f6a7405128ab5b729fb8d
---

diff --git a/src/coreclr/src/gc/env/gcenv.os.h b/src/coreclr/src/gc/env/gcenv.os.h
index 6ea35e3..d3e40ac 100644
--- a/src/coreclr/src/gc/env/gcenv.os.h
+++ b/src/coreclr/src/gc/env/gcenv.os.h
@@ -50,6 +50,14 @@ struct GCThreadAffinity
 // An event is a synchronization object whose state can be set and reset
 // indicating that an event has occured. It is used pervasively throughout
 // the GC.
+//
+// Note that GCEvent deliberately leaks its contents by not having a non-trivial destructor.
+// This is by design; since all uses of GCEvent have static lifetime, their destructors
+// are run on process exit, potentially concurrently with other threads that may still be
+// operating on the static event. To avoid these sorts of unsafety, GCEvent chooses to
+// not have a destructor at all. The cost of this is leaking a small amount of memory, but
+// this is not a problem since a majority of the uses of GCEvent are static. See CoreCLR#11111
+// for more details on the hazards of static destructors.
 class GCEvent {
 private:
     class Impl;
@@ -59,9 +67,6 @@ public:
     // Constructs a new uninitialized event.
     GCEvent();
 
-    // Destructs an event.
-    ~GCEvent();
-
     // Closes the event. Attempting to use the event past calling CloseEvent
     // is a logic error.
     void CloseEvent();
diff --git a/src/coreclr/src/gc/unix/events.cpp b/src/coreclr/src/gc/unix/events.cpp
index f51eae8..7c665f4 100644
--- a/src/coreclr/src/gc/unix/events.cpp
+++ b/src/coreclr/src/gc/unix/events.cpp
@@ -247,12 +247,6 @@ GCEvent::GCEvent()
 {
 }
 
-GCEvent::~GCEvent()
-{
-    delete m_impl;
-    m_impl = nullptr;
-}
-
 void GCEvent::CloseEvent()
 {
     assert(m_impl != nullptr);
diff --git a/src/coreclr/src/gc/windows/gcenv.windows.cpp b/src/coreclr/src/gc/windows/gcenv.windows.cpp
index 0f3fd71..3749f06 100644
--- a/src/coreclr/src/gc/windows/gcenv.windows.cpp
+++ b/src/coreclr/src/gc/windows/gcenv.windows.cpp
@@ -690,12 +690,6 @@ GCEvent::GCEvent()
 {
 }
 
-GCEvent::~GCEvent()
-{
-    delete m_impl;
-    m_impl = nullptr;
-}
-
 void GCEvent::CloseEvent()
 {
     assert(m_impl != nullptr);
diff --git a/src/coreclr/src/vm/gcenv.os.cpp b/src/coreclr/src/vm/gcenv.os.cpp
index 5e820a6..77be88c 100644
--- a/src/coreclr/src/vm/gcenv.os.cpp
+++ b/src/coreclr/src/vm/gcenv.os.cpp
@@ -793,12 +793,6 @@ GCEvent::GCEvent()
 {
 }
 
-GCEvent::~GCEvent()
-{
-    delete m_impl;
-    m_impl = nullptr;
-}
-
 void GCEvent::CloseEvent()
 {
     WRAPPER_NO_CONTRACT;