From 521473ee4bde8e1697211de30a1aa125cdc35ff8 Mon Sep 17 00:00:00 2001
From: whitequark <whitequark@whitequark.org>
Date: Sun, 10 May 2020 03:37:52 +0000
Subject: [PATCH] Win32: use CRT heap instead of our own.

Historically SolveSpace used its own heap on Windows since it gave
better control and debugging options, but a lot of development these
days happens on Linux, where that heap was a stub around malloc/free,
and also Windows debugging tools got a lot better.

In terms of immediate benefit, this commit fixes heap corruption
on Windows introduced in commits b4e1ce44 and 47e82798, caused
by allocating with HEAP_NO_SERIALIZE in parallel from OpenMP threads.
Without HEAP_NO_SERIALIZE there's no performance benefit to keeping
our own heap, either.

The vl() function is also removed because for development there are
better tools now, and the only place where it was permanently called
from became a no-op, since temporary heap always validates after
FreeAllTemporary() recreates it.
---
 CMakeLists.txt           |  3 ++-
 src/platform/utilwin.cpp | 16 +++-------------
 src/solvespace.h         |  1 -
 3 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4f5eab56..6494993f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -98,7 +98,8 @@ endif()
 
 # We use OpenMP to speed up some geometric operations, but it's optional.
 include(FindOpenMP)
-if(OpenMP_FOUND)
+# No MinGW distribution actually ships an OpenMP runtime, but FindOpenMP detects it anyway.
+if(OpenMP_FOUND AND NOT MINGW)
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
 else()
     message(WARNING "OpenMP not found, geometric operations will be single-threaded")
diff --git a/src/platform/utilwin.cpp b/src/platform/utilwin.cpp
index 4e452802..172039ae 100644
--- a/src/platform/utilwin.cpp
+++ b/src/platform/utilwin.cpp
@@ -13,7 +13,7 @@
 #include <shellapi.h>
 
 namespace SolveSpace {
-static HANDLE PermHeap, TempHeap;
+static HANDLE TempHeap;
 
 void dbp(const char *str, ...)
 {
@@ -51,23 +51,15 @@ void FreeAllTemporary()
 {
     if(TempHeap) HeapDestroy(TempHeap);
     TempHeap = HeapCreate(HEAP_NO_SERIALIZE, 1024*1024*20, 0);
-    // This is a good place to validate, because it gets called fairly
-    // often.
-    vl();
 }
 
 void *MemAlloc(size_t n) {
-    void *p = HeapAlloc(PermHeap, HEAP_NO_SERIALIZE | HEAP_ZERO_MEMORY, n);
+    void *p = malloc(n);
     ssassert(p != NULL, "Cannot allocate memory");
     return p;
 }
 void MemFree(void *p) {
-    HeapFree(PermHeap, HEAP_NO_SERIALIZE, p);
-}
-
-void vl() {
-    ssassert(HeapValidate(TempHeap, HEAP_NO_SERIALIZE, NULL), "Corrupted heap");
-    ssassert(HeapValidate(PermHeap, HEAP_NO_SERIALIZE, NULL), "Corrupted heap");
+    free(p);
 }
 
 std::vector<std::string> InitPlatform(int argc, char **argv) {
@@ -81,8 +73,6 @@ std::vector<std::string> InitPlatform(int argc, char **argv) {
     }
 #endif
 
-    // Create the heap used for long-lived stuff (that gets freed piecewise).
-    PermHeap = HeapCreate(HEAP_NO_SERIALIZE, 1024*1024*20, 0);
     // Create the heap that we use to store Exprs and other temp stuff.
     FreeAllTemporary();
 
diff --git a/src/solvespace.h b/src/solvespace.h
index 2a553024..fea705e8 100644
--- a/src/solvespace.h
+++ b/src/solvespace.h
@@ -152,7 +152,6 @@ void *AllocTemporary(size_t n);
 void FreeAllTemporary();
 void *MemAlloc(size_t n);
 void MemFree(void *p);
-void vl(); // debug function to validate heaps
 
 // End of platform-specific functions
 //================