From 6a9de27c880b0ffc02172895392975dacd1085f7 Mon Sep 17 00:00:00 2001
From: Venkata krishnan Sowrirajan <vsowrirajan@qubole.com>
Date: Wed, 27 Mar 2019 19:19:18 -0700
Subject: [PATCH 1/2] Fix deadlock while freeing page

---
 .../unsafe/sort/UnsafeExternalSorter.java     | 29 ++++++++++++-------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorter.java b/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorter.java
index af5a934b7da6..86cd61cec760 100644
--- a/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorter.java
+++ b/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorter.java
@@ -573,19 +573,26 @@ public boolean hasNext() {
 
     @Override
     public void loadNext() throws IOException {
-      synchronized (this) {
-        loaded = true;
-        if (nextUpstream != null) {
-          // Just consumed the last record from in memory iterator
-          if (lastPage != null) {
-            freePage(lastPage);
-            lastPage = null;
+      MemoryBlock pageToFree = null;
+      try {
+        synchronized (this) {
+          loaded = true;
+          if (nextUpstream != null) {
+            // Just consumed the last record from in memory iterator
+            if(lastPage != null) {
+              pageToFree = lastPage;
+              lastPage = null;
+            }
+            upstream = nextUpstream;
+            nextUpstream = null;
           }
-          upstream = nextUpstream;
-          nextUpstream = null;
+          numRecords--;
+          upstream.loadNext();
+        }
+      } finally {
+        if (pageToFree != null) {
+          freePage(pageToFree);
         }
-        numRecords--;
-        upstream.loadNext();
       }
     }
 

From 229cfaeb56e1268f4b83f3a075be1027cc33a870 Mon Sep 17 00:00:00 2001
From: Venkata krishnan Sowrirajan <vsowrirajan@qubole.com>
Date: Wed, 3 Apr 2019 09:33:31 -0700
Subject: [PATCH 2/2] Addressed review comments

---
 .../util/collection/unsafe/sort/UnsafeExternalSorter.java    | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorter.java b/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorter.java
index 86cd61cec760..231451468320 100644
--- a/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorter.java
+++ b/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorter.java
@@ -580,6 +580,11 @@ public void loadNext() throws IOException {
           if (nextUpstream != null) {
             // Just consumed the last record from in memory iterator
             if(lastPage != null) {
+              // Do not free the page here, while we are locking `SpillableIterator`. The `freePage`
+              // method locks the `TaskMemoryManager`, and it's a bad idea to lock 2 objects in
+              // sequence. We may hit dead lock if another thread locks `TaskMemoryManager` and
+              // `SpillableIterator` in sequence, which may happen in
+              // `TaskMemoryManager.acquireExecutionMemory`. 
               pageToFree = lastPage;
               lastPage = null;
             }