Reland "Stop trying to destroy PartitionAlloc" This is a reland of commit f8e0f053178d9030b9e3878d780bc75f70518e1c Avoid exit-time destructors this time. Original change's description: > Stop trying to destroy PartitionAlloc > > Attempting to destroy and reinitialize PartitionAlloc results in > PartitionAlloc leaking address space. So destroying PartitionAlloc to > "fix a memory leak" actually makes the situation worse in PA-enabled > builds where PDFium gets repeatedly destroyed and reinitialized. > > Delete FX_InitializeMemoryAllocators() and FX_DestroyMemoryAllocators() > and update their callers. Then make the PartitionAllocator instances > initialized on first use again. This approximately reverts > https://pdfium-review.googlesource.com/116751 without re-introducing > NoDestructor usage. > > Change FPDFViewEmbedderTest.RepeatedInitDestroy to loop more than 4096 > times to show this works. While only changing the test, as seen in > https://pdfium-review.googlesource.com/144731, results in > pdfium_embeddertests crashes. > > Bug: 42270890 > Change-Id: I79f9c2fa5adf0e0f9c8b8bf30332c96966ed2c96 > Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/145070 > Commit-Queue: Lei Zhang <thestig@chromium.org> > Reviewed-by: Tom Sepez <tsepez@chromium.org> Bug: 42270890 Change-Id: I53b91f6957783b33c1074b839d22b7922e4cfd5e Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/145111 Commit-Queue: Lei Zhang <thestig@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fxcrt/fx_memory.h b/core/fxcrt/fx_memory.h index fa7132c..9191dc8 100644 --- a/core/fxcrt/fx_memory.h +++ b/core/fxcrt/fx_memory.h
@@ -30,8 +30,6 @@ #include <stdlib.h> #endif -void FX_InitializeMemoryAllocators(); -void FX_DestroyMemoryAllocators(); NOINLINE void FX_OutOfMemoryTerminate(size_t size); // General Partition Allocators.
diff --git a/core/fxcrt/fx_memory_malloc.cpp b/core/fxcrt/fx_memory_malloc.cpp index 93cc7c6..82bb273 100644 --- a/core/fxcrt/fx_memory_malloc.cpp +++ b/core/fxcrt/fx_memory_malloc.cpp
@@ -69,7 +69,3 @@ } // namespace internal } // namespace pdfium - -void FX_InitializeMemoryAllocators() {} - -void FX_DestroyMemoryAllocators() {}
diff --git a/core/fxcrt/fx_memory_pa.cpp b/core/fxcrt/fx_memory_pa.cpp index 52dccbd..43f5d7f 100644 --- a/core/fxcrt/fx_memory_pa.cpp +++ b/core/fxcrt/fx_memory_pa.cpp
@@ -19,19 +19,16 @@ constexpr partition_alloc::PartitionOptions kOptions = {}; -struct Allocators { - partition_alloc::PartitionAllocator general_allocator{kOptions}; - partition_alloc::PartitionAllocator string_allocator{kOptions}; -}; - -Allocators* g_allocators = nullptr; - partition_alloc::PartitionAllocator& GetGeneralPartitionAllocator() { - return g_allocators->general_allocator; + static auto* general_allocator = + new partition_alloc::PartitionAllocator{kOptions}; + return *general_allocator; } partition_alloc::PartitionAllocator& GetStringPartitionAllocator() { - return g_allocators->string_allocator; + static auto* string_allocator = + new partition_alloc::PartitionAllocator{kOptions}; + return *string_allocator; } } // namespace @@ -110,14 +107,3 @@ } } // namespace pdfium::internal - -void FX_InitializeMemoryAllocators() { - if (!g_allocators) { - g_allocators = new Allocators(); - } -} - -void FX_DestroyMemoryAllocators() { - delete g_allocators; - g_allocators = nullptr; -}
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp index f747fe1..c808459 100644 --- a/fpdfsdk/fpdf_view.cpp +++ b/fpdfsdk/fpdf_view.cpp
@@ -197,7 +197,6 @@ if (g_bLibraryInitialized) { return; } - FX_InitializeMemoryAllocators(); CFX_Timer::InitializeGlobals(); CFX_GEModule::RendererType renderer_type = @@ -261,7 +260,6 @@ pdfium::DestroyPageModule(); CFX_GEModule::Destroy(); CFX_Timer::DestroyGlobals(); - FX_DestroyMemoryAllocators(); g_bLibraryInitialized = false; }
diff --git a/fpdfsdk/fpdf_view_embeddertest.cpp b/fpdfsdk/fpdf_view_embeddertest.cpp index 7235771..833fb8f 100644 --- a/fpdfsdk/fpdf_view_embeddertest.cpp +++ b/fpdfsdk/fpdf_view_embeddertest.cpp
@@ -487,7 +487,7 @@ } TEST_F(FPDFViewEmbedderTest, RepeatedInitDestroy) { - for (int i = 0; i < 3; ++i) { + for (int i = 0; i < 4400; ++i) { if (!OpenDocument("about_blank.pdf")) { ADD_FAILURE(); }
diff --git a/testing/embedder_test_main.cpp b/testing/embedder_test_main.cpp index 41342dd..d220e31 100644 --- a/testing/embedder_test_main.cpp +++ b/testing/embedder_test_main.cpp
@@ -3,7 +3,6 @@ // found in the LICENSE file. #include "build/build_config.h" -#include "core/fxcrt/fx_memory.h" #include "testing/embedder_test_environment.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -28,8 +27,6 @@ pdfium::ConfigurePartitionAllocShimPartitionForTest(); #endif - FX_InitializeMemoryAllocators(); - #ifdef PDF_ENABLE_V8 // The env will be deleted by gtest. AddGlobalTestEnvironment(new V8TestEnvironment(argv[0]));
diff --git a/testing/image_diff/image_diff.cpp b/testing/image_diff/image_diff.cpp index 035e12b..3e3e3d2 100644 --- a/testing/image_diff/image_diff.cpp +++ b/testing/image_diff/image_diff.cpp
@@ -17,7 +17,6 @@ #include <string> #include <vector> -#include "core/fxcrt/fx_memory.h" #include "core/fxcrt/numerics/safe_conversions.h" #include "testing/image_diff/image_diff_png.h" #include "testing/utils/path_service.h" @@ -400,8 +399,6 @@ } int main(int argc, const char* argv[]) { - FX_InitializeMemoryAllocators(); - bool histograms = false; bool produce_diff_image = false; bool produce_image_subtraction = false;
diff --git a/testing/unit_test_main.cpp b/testing/unit_test_main.cpp index 28476b0..af254cb 100644 --- a/testing/unit_test_main.cpp +++ b/testing/unit_test_main.cpp
@@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "core/fxcrt/fx_memory.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/pdf_test_environment.h" @@ -25,8 +24,6 @@ pdfium::ConfigurePartitionAllocShimPartitionForTest(); #endif // defined(PDF_USE_PARTITION_ALLOC) - FX_InitializeMemoryAllocators(); - // PDF test environment will be deleted by gtest. AddGlobalTestEnvironment(new PDFTestEnvironment());