From f4a25f642991a7114b86f6eb7d0bac3d599953a6 Mon Sep 17 00:00:00 2001
From: Ben Nemec <bnemec@redhat.com>
Date: Tue, 26 Feb 2019 22:12:23 +0000
Subject: [PATCH] Fix memcache pool client in monkey-patched environments

First off, this is an ugly hack, but we're dealing with code that
essentially monkey-patches a monkey-patch. You reap what you sow.

Per the linked bug, our connection pool client explodes on python 3
with eventlet monkey-patching in force:

TypeError: object() takes no parameters

This is due to the way __new__ is overridden in the class. We need
to strip arguments from the call before they get to object(), which
doesn't accept args.

Unfortunately, when we're _not_ monkey-patched, adding the new
override implementation fails with:

TypeError: object.__new__(_MemcacheClient) is not safe,
use Client.__new__()

As such, we need different implementations depending on whether we
are monkey-patched or not. This change passes both with and without
monkey-patching and adds a unit test that exposes the bug.

Note that this is a temporary, backportable fix that will ultimately
be replaced by a switch to the pymemcache library which does not
have the threading.local problem being worked around here.

Change-Id: I039dffadeebd0ff4479b9c870c257772c43aba53
Partial-Bug: 1812935
---
 oslo_cache/_memcache_pool.py             | 14 +++++++++++++-
 oslo_cache/tests/test_connection_pool.py | 17 +++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/oslo_cache/_memcache_pool.py b/oslo_cache/_memcache_pool.py
index bd1a5296..61501d16 100644
--- a/oslo_cache/_memcache_pool.py
+++ b/oslo_cache/_memcache_pool.py
@@ -24,6 +24,10 @@ import itertools
 import threading
 import time
 
+try:
+    import eventlet
+except ImportError:
+    eventlet = None
 import memcache
 from oslo_log import log
 from six.moves import queue
@@ -45,9 +49,17 @@ class _MemcacheClient(memcache.Client):
     """
     __delattr__ = object.__delattr__
     __getattribute__ = object.__getattribute__
-    __new__ = object.__new__
     __setattr__ = object.__setattr__
 
+    # Hack for lp 1812935
+    if eventlet and eventlet.patcher.is_monkey_patched('thread'):
+        # NOTE(bnemec): I'm not entirely sure why this works in a
+        # monkey-patched environment and not with vanilla stdlib, but it does.
+        def __new__(cls, *args, **kwargs):
+            return object.__new__(cls)
+    else:
+        __new__ = object.__new__
+
     def __del__(self):
         pass
 
diff --git a/oslo_cache/tests/test_connection_pool.py b/oslo_cache/tests/test_connection_pool.py
index 07ea7f14..1351937e 100644
--- a/oslo_cache/tests/test_connection_pool.py
+++ b/oslo_cache/tests/test_connection_pool.py
@@ -145,3 +145,20 @@ class TestMemcacheClientOverrides(test_cache.BaseTestCase):
             if field not in ('__dict__', '__weakref__'):
                 self.assertNotEqual(id(getattr(thread_local, field, None)),
                                     id(getattr(client_class, field, None)))
+
+    def test_can_create_with_kwargs(self):
+        """Test for lp 1812935
+
+        Note that in order to reproduce the bug, it is necessary to add the
+        following to the top of oslo_cache/tests/__init__.py::
+
+            import eventlet
+            eventlet.monkey_patch()
+
+        This should happen before any other imports in that file.
+        """
+        client = _memcache_pool._MemcacheClient('foo', check_keys=False)
+        # Make sure kwargs are properly processed by the client
+        self.assertFalse(client.do_check_key)
+        # Make sure our __new__ override still results in the right type
+        self.assertIsInstance(client, _memcache_pool._MemcacheClient)