Remove destructor from GCEvent and instead rely on the OS to clean up (#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

diff --git a/src/gc/env/gcenv.os.h b/src/gc/env/gcenv.os.h
index 6ea35e3..d3e40ac 100644 (file)
--- a/src/gc/env/gcenv.os.h
+++ b/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/gc/unix/events.cpp b/src/gc/unix/events.cpp
index f51eae8..7c665f4 100644 (file)
--- a/src/gc/unix/events.cpp
+++ b/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/gc/windows/gcenv.windows.cpp b/src/gc/windows/gcenv.windows.cpp
index 0f3fd71..3749f06 100644 (file)
--- a/src/gc/windows/gcenv.windows.cpp
+++ b/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/vm/gcenv.os.cpp b/src/vm/gcenv.os.cpp
index 5e820a6..77be88c 100644 (file)
--- a/src/vm/gcenv.os.cpp
+++ b/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;