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
Closes-Bug: 1812672
(cherry picked from commit f4a25f6429)
This commit is contained in:
Ben Nemec 2019-02-26 22:12:23 +00:00 committed by Ben Nemec
parent 5f42092e2c
commit caf5443de8
2 changed files with 30 additions and 1 deletions

View File

@ -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

View File

@ -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)