From 0640758b7d56b214ee379f99673c506dd39fd230 Mon Sep 17 00:00:00 2001 From: Sean Gillespie Date: Sat, 22 Apr 2017 04:39:09 -0700 Subject: [PATCH] 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 --- src/coreclr/src/gc/env/gcenv.os.h | 11 ++++++++--- src/coreclr/src/gc/unix/events.cpp | 6 ------ src/coreclr/src/gc/windows/gcenv.windows.cpp | 6 ------ src/coreclr/src/vm/gcenv.os.cpp | 6 ------ 4 files changed, 8 insertions(+), 21 deletions(-) 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; -- 2.7.4