From 0ffa99c56ba6f8f3c472d77176a1be809078b270 Mon Sep 17 00:00:00 2001
From: Michael Graeb <graebm@amazon.com>
Date: Wed, 16 Oct 2019 17:23:41 -0700
Subject: [PATCH] Fix mysterious python 2 crash (#87)

- fix mysterious python 2 crash
- add regression test for mysterious python 2 crash
- fix actual mis-use of PyWeakref_GetObject()
- bump version#
---
 setup.py                 |  2 +-
 source/http_message.c    |  3 ++-
 source/http_stream.c     |  7 ++++++-
 test/__init__.py         |  5 +++++
 test/test_http_client.py | 24 ++++++++++++++++++++++++
 5 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/setup.py b/setup.py
index 251bd8df5..d59001aa3 100644
--- a/setup.py
+++ b/setup.py
@@ -241,7 +241,7 @@ def awscrt_ext():
 
 setuptools.setup(
     name="awscrt",
-    version="0.3.1",
+    version="0.3.2",
     author="Amazon Web Services, Inc",
     author_email="aws-sdk-common-runtime@amazon.com",
     description="A common runtime for AWS Python projects",
diff --git a/source/http_message.c b/source/http_message.c
index d8f12acce..6a4f1a737 100644
--- a/source/http_message.c
+++ b/source/http_message.c
@@ -67,7 +67,8 @@ static bool s_http_message_update_from_py(struct http_message_binding *message)
 
     /* Doing a lot of queries so grab a hard reference */
     message_self = PyWeakref_GetObject(message->self_proxy);
-    if (!message_self) {
+    if (message_self == Py_None) {
+        PyErr_SetString(PyExc_RuntimeError, "HttpMessageBase destroyed");
         goto done;
     }
     Py_INCREF(message_self);
diff --git a/source/http_stream.c b/source/http_stream.c
index 1860fcde6..7358b3c35 100644
--- a/source/http_stream.c
+++ b/source/http_stream.c
@@ -219,7 +219,12 @@ static void s_on_stream_complete(struct aws_http_stream *native_stream, int erro
     Py_DECREF(result);
 
     /* DECREF python self, we don't need to force it to stay alive any longer. */
-    Py_DECREF(PyWeakref_GetObject(stream->self_proxy));
+    PyObject *self = PyWeakref_GetObject(stream->self_proxy);
+    Py_DECREF(self);
+    /* Hack note. This use to be a one liner: Py_DECREF(PyWeakref_GetObject(stream->self_proxy));
+     * that would crash on Python 2, saying we were DECREF'ing Py_None.
+     * That should be impossible because we're forcing the python self to stay alive.
+     * I have no idea why splitting it into 2 lines fixes everything, but it does. */
 
     PyGILState_Release(state);
     /*************** GIL RELEASE ***************/
diff --git a/test/__init__.py b/test/__init__.py
index 492158187..bb2a1622c 100644
--- a/test/__init__.py
+++ b/test/__init__.py
@@ -16,6 +16,7 @@
 import gc
 import inspect
 import sys
+import time
 import types
 import unittest
 
@@ -31,6 +32,10 @@ def setUp(self):
     def tearDown(self):
         gc.collect()
 
+        # Native resources might need a few more ticks to finish cleaning themselves up.
+        if NativeResource._living:
+            time.sleep(1)
+
         # Print out debugging info on leaking resources
         if NativeResource._living:
             print('Leaking NativeResources:')
diff --git a/test/test_http_client.py b/test/test_http_client.py
index c6d628790..b8fa611bb 100644
--- a/test/test_http_client.py
+++ b/test/test_http_client.py
@@ -227,6 +227,30 @@ def test_put_http(self):
     def test_put_https(self):
         self._test_put(secure=True)
 
+    # Ensure that stream and connection classes stay alive until work is complete
+    def _test_stream_lives_until_complete(self, secure):
+        self._start_server(secure)
+        connection = self._new_client_connection(secure)
+
+        request = HttpRequest('GET', '/test/test_http_client.py')
+        stream = connection.request(request)
+        completion_future = stream.completion_future
+
+        # delete all local references
+        del stream
+        del connection
+
+        # stream should still complete successfully
+        completion_future.result(self.timeout)
+
+        self._stop_server()
+
+    def test_stream_lives_until_complete_http(self):
+        self._test_stream_lives_until_complete(secure=False)
+
+    def test_stream_lives_until_complete_https(self):
+        self._test_stream_lives_until_complete(secure=True)
+
 
 if __name__ == '__main__':
     unittest.main()