Remove NoDestructor usage in timer classes
Add static methods to initialize/destroy 2 different timer classes. This
lets FPDF_DestroyLibrary() free more memory and work as it was
originally intended.
Bug: pdfium:1876
Change-Id: I8de78e10ebcdd9dabd168b1dc66619f402fca135
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/113112
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fxcrt/cfx_timer.cpp b/core/fxcrt/cfx_timer.cpp
index 744b1eb..5f2a931 100644
--- a/core/fxcrt/cfx_timer.cpp
+++ b/core/fxcrt/cfx_timer.cpp
@@ -9,18 +9,26 @@
#include <map>
#include "third_party/base/check.h"
-#include "third_party/base/no_destructor.h"
namespace {
using TimerMap = std::map<int32_t, CFX_Timer*>;
-TimerMap& GetPWLTimerMap() {
- static pdfium::base::NoDestructor<TimerMap> timer_map;
- return *timer_map;
-}
+TimerMap* g_pwl_timer_map = nullptr;
} // namespace
+// static
+void CFX_Timer::InitializeGlobals() {
+ CHECK(!g_pwl_timer_map);
+ g_pwl_timer_map = new TimerMap();
+}
+
+// static
+void CFX_Timer::DestroyGlobals() {
+ delete g_pwl_timer_map;
+ g_pwl_timer_map = nullptr;
+}
+
CFX_Timer::CFX_Timer(HandlerIface* pHandlerIface,
CallbackIface* pCallbackIface,
int32_t nInterval)
@@ -29,13 +37,13 @@
if (m_pHandlerIface) {
m_nTimerID = m_pHandlerIface->SetTimer(nInterval, TimerProc);
if (HasValidID())
- GetPWLTimerMap()[m_nTimerID] = this;
+ (*g_pwl_timer_map)[m_nTimerID] = this;
}
}
CFX_Timer::~CFX_Timer() {
if (HasValidID()) {
- GetPWLTimerMap().erase(m_nTimerID);
+ g_pwl_timer_map->erase(m_nTimerID);
if (m_pHandlerIface)
m_pHandlerIface->KillTimer(m_nTimerID);
}
@@ -43,7 +51,8 @@
// static
void CFX_Timer::TimerProc(int32_t idEvent) {
- auto it = GetPWLTimerMap().find(idEvent);
- if (it != GetPWLTimerMap().end())
+ auto it = g_pwl_timer_map->find(idEvent);
+ if (it != g_pwl_timer_map->end()) {
it->second->m_pCallbackIface->OnTimerFired();
+ }
}
diff --git a/core/fxcrt/cfx_timer.h b/core/fxcrt/cfx_timer.h
index 24ed296..e8be792 100644
--- a/core/fxcrt/cfx_timer.h
+++ b/core/fxcrt/cfx_timer.h
@@ -35,6 +35,9 @@
virtual void OnTimerFired() = 0;
};
+ static void InitializeGlobals();
+ static void DestroyGlobals();
+
CFX_Timer(HandlerIface* pHandlerIface,
CallbackIface* pCallbackIface,
int32_t nInterval);
diff --git a/core/fxcrt/cfx_timer_unittest.cpp b/core/fxcrt/cfx_timer_unittest.cpp
index 32220f0..360dbb2 100644
--- a/core/fxcrt/cfx_timer_unittest.cpp
+++ b/core/fxcrt/cfx_timer_unittest.cpp
@@ -14,6 +14,8 @@
using testing::Return;
using testing::SaveArg;
+namespace {
+
class MockTimerScheduler : public CFX_Timer::HandlerIface {
public:
MOCK_METHOD2(SetTimer, int(int32_t uElapse, TimerCallback lpTimerFunc));
@@ -25,7 +27,14 @@
MOCK_METHOD0(OnTimerFired, void());
};
-TEST(CFX_Timer, ValidTimers) {
+} // namespace
+
+class CFXTimer : public testing::Test {
+ void SetUp() override { CFX_Timer::InitializeGlobals(); }
+ void TearDown() override { CFX_Timer::DestroyGlobals(); }
+};
+
+TEST_F(CFXTimer, ValidTimers) {
CFX_Timer::HandlerIface::TimerCallback fn1 = nullptr;
CFX_Timer::HandlerIface::TimerCallback fn2 = nullptr;
@@ -56,7 +65,7 @@
(*fn1)(1002);
}
-TEST(CFX_Timer, MisbehavingEmbedder) {
+TEST_F(CFXTimer, MisbehavingEmbedder) {
CFX_Timer::HandlerIface::TimerCallback fn1 = nullptr;
MockTimerScheduler scheduler;
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index d6a58cc..fbe75d9 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -31,6 +31,7 @@
#include "core/fpdfdoc/cpdf_nametree.h"
#include "core/fpdfdoc/cpdf_viewerpreferences.h"
#include "core/fxcrt/cfx_read_only_span_stream.h"
+#include "core/fxcrt/cfx_timer.h"
#include "core/fxcrt/fx_safe_types.h"
#include "core/fxcrt/fx_stream.h"
#include "core/fxcrt/fx_system.h"
@@ -229,6 +230,7 @@
return;
FX_InitializeMemoryAllocators();
+ CFX_Timer::InitializeGlobals();
CFX_GEModule::Create(config ? config->m_pUserFontPaths : nullptr);
CPDF_PageModule::Create();
@@ -261,6 +263,7 @@
CPDF_PageModule::Destroy();
CFX_GEModule::Destroy();
+ CFX_Timer::DestroyGlobals();
g_bLibraryInitialized = false;
}
diff --git a/fxjs/global_timer.cpp b/fxjs/global_timer.cpp
index 035aff8..e54e635 100644
--- a/fxjs/global_timer.cpp
+++ b/fxjs/global_timer.cpp
@@ -12,18 +12,26 @@
#include "fxjs/cjs_app.h"
#include "third_party/base/check.h"
#include "third_party/base/containers/contains.h"
-#include "third_party/base/no_destructor.h"
namespace {
using TimerMap = std::map<int32_t, GlobalTimer*>;
-TimerMap& GetGlobalTimerMap() {
- static pdfium::base::NoDestructor<TimerMap> timer_map;
- return *timer_map;
-}
+TimerMap* g_global_timer_map = nullptr;
} // namespace
+// static
+void GlobalTimer::InitializeGlobals() {
+ CHECK(!g_global_timer_map);
+ g_global_timer_map = new TimerMap();
+}
+
+// static
+void GlobalTimer::DestroyGlobals() {
+ delete g_global_timer_map;
+ g_global_timer_map = nullptr;
+}
+
GlobalTimer::GlobalTimer(CJS_App* pObj,
CJS_Runtime* pRuntime,
Type nType,
@@ -37,8 +45,8 @@
m_pRuntime(pRuntime),
m_pEmbedApp(pObj) {
if (HasValidID()) {
- DCHECK(!pdfium::Contains(GetGlobalTimerMap(), m_nTimerID));
- GetGlobalTimerMap()[m_nTimerID] = this;
+ DCHECK(!pdfium::Contains((*g_global_timer_map), m_nTimerID));
+ (*g_global_timer_map)[m_nTimerID] = this;
}
}
@@ -49,15 +57,16 @@
if (m_pRuntime && m_pRuntime->GetTimerHandler())
m_pRuntime->GetTimerHandler()->KillTimer(m_nTimerID);
- DCHECK(pdfium::Contains(GetGlobalTimerMap(), m_nTimerID));
- GetGlobalTimerMap().erase(m_nTimerID);
+ DCHECK(pdfium::Contains((*g_global_timer_map), m_nTimerID));
+ g_global_timer_map->erase(m_nTimerID);
}
// static
void GlobalTimer::Trigger(int32_t nTimerID) {
- auto it = GetGlobalTimerMap().find(nTimerID);
- if (it == GetGlobalTimerMap().end())
+ auto it = g_global_timer_map->find(nTimerID);
+ if (it == g_global_timer_map->end()) {
return;
+ }
GlobalTimer* pTimer = it->second;
if (pTimer->m_bProcessing)
@@ -68,9 +77,10 @@
pTimer->m_pEmbedApp->TimerProc(pTimer);
// Timer proc may have destroyed timer, find it again.
- it = GetGlobalTimerMap().find(nTimerID);
- if (it == GetGlobalTimerMap().end())
+ it = g_global_timer_map->find(nTimerID);
+ if (it == g_global_timer_map->end()) {
return;
+ }
pTimer = it->second;
pTimer->m_bProcessing = false;
@@ -80,9 +90,10 @@
// static
void GlobalTimer::Cancel(int32_t nTimerID) {
- auto it = GetGlobalTimerMap().find(nTimerID);
- if (it == GetGlobalTimerMap().end())
+ auto it = g_global_timer_map->find(nTimerID);
+ if (it == g_global_timer_map->end()) {
return;
+ }
GlobalTimer* pTimer = it->second;
pTimer->m_pEmbedApp->CancelProc(pTimer);
diff --git a/fxjs/global_timer.h b/fxjs/global_timer.h
index 106c9e6..53a67b7 100644
--- a/fxjs/global_timer.h
+++ b/fxjs/global_timer.h
@@ -19,6 +19,9 @@
kOneShot = true,
};
+ static void InitializeGlobals();
+ static void DestroyGlobals();
+
GlobalTimer(CJS_App* pObj,
CJS_Runtime* pRuntime,
Type nType,
diff --git a/fxjs/ijs_runtime.cpp b/fxjs/ijs_runtime.cpp
index 0308524..11c90fd 100644
--- a/fxjs/ijs_runtime.cpp
+++ b/fxjs/ijs_runtime.cpp
@@ -10,6 +10,7 @@
#include "fpdfsdk/cpdfsdk_formfillenvironment.h"
#include "fxjs/cfxjs_engine.h"
#include "fxjs/cjs_runtime.h"
+#include "fxjs/global_timer.h"
#ifdef PDF_ENABLE_XFA
#include "fxjs/gc/heap.h"
#endif // PDF_ENABLE_XFA
@@ -25,6 +26,7 @@
// static
void IJS_Runtime::Initialize(unsigned int slot, void* isolate, void* platform) {
#ifdef PDF_ENABLE_V8
+ GlobalTimer::InitializeGlobals();
FXJS_Initialize(slot, static_cast<v8::Isolate*>(isolate));
#ifdef PDF_ENABLE_XFA
FXGC_Initialize(static_cast<v8::Platform*>(platform),
@@ -40,6 +42,7 @@
FXGC_Release();
#endif // PDF_ENABLE_XFA
FXJS_Release();
+ GlobalTimer::DestroyGlobals();
#endif // PDF_ENABLE_V8
}
