Change many important assert statements to explicit if tests so they get...

Change many important assert statements to explicit if tests so they get executed even when optimizations are enabled with -O and errors and impossible states don't go on silently undetected possibly losing data.

changelog.rst

@@ -7,7 +7,11 @@
 1.1b4 (Unreleased)
 ==================
 
-- TBD
+- Detect and raise an error for several important types of
+  programming errors even if Python interpreter optimizations are
+  enabled with ``-O`` or ``PYTHONOPTIMIZE``. Previously these would go
+  undetected if optimizations were enabled, potentially leading to
+  erratic, difficult to debug behaviour.
 
 1.1b3 (Aug 16, 2015)
 ====================

gevent/_fileobjectposix.py

@@ -145,13 +145,19 @@ class FileObjectPosix(object):
         if not isinstance(fileno, int):
             raise TypeError('fileno must be int: %r' % fileno)
 
+        orig_mode = mode
         mode = (mode or 'rb').replace('b', '')
         if 'U' in mode:
             self._translate = True
             mode = mode.replace('U', '')
         else:
             self._translate = False
-        assert len(mode) == 1, 'mode can only be [rb, rU, wb]'
+        if len(mode) != 1:
+            # Python 3 builtin `open` raises a ValueError for invalid modes;
+            # Python 2 ignores in. In the past, we raised an AssertionError, if __debug__ was
+            # enabled (which it usually was). Match Python 3 because it makes more sense
+            # and because __debug__ may not be enabled
+            raise ValueError('mode can only be [rb, rU, wb], not %r' % (orig_mode,))
 
         self._fobj = fobj
         self._closed = False

gevent/_socket2.py

@@ -142,7 +142,8 @@ class socket(object):
 
         If :func:`cancel_wait` is called, raise ``socket.error(EBADF, 'File descriptor was closed in another greenlet')``.
         """
-        assert watcher.callback is None, 'This socket is already used by another greenlet: %r' % (watcher.callback, )
+        if watcher.callback is not None:
+            raise _socketcommon.ConcurrentObjectUseError('This socket is already used by another greenlet: %r' % (watcher.callback, ))
         if self.timeout is not None:
             timeout = Timeout.start_new(self.timeout, timeout_exc, ref=False)
         else:

gevent/_socket3.py

@@ -116,7 +116,8 @@ class socket(object):
 
         If :func:`cancel_wait` is called, raise ``socket.error(EBADF, 'File descriptor was closed in another greenlet')``.
         """
-        assert watcher.callback is None, 'This socket is already used by another greenlet: %r' % (watcher.callback, )
+        if watcher.callback is not None:
+            raise _socketcommon.ConcurrentObjectUseError('This socket is already used by another greenlet: %r' % (watcher.callback, ))
         if self.timeout is not None:
             timeout = Timeout.start_new(self.timeout, timeout_exc, ref=False)
         else:

gevent/_socketcommon.py

@@ -62,6 +62,7 @@ __imports__.extend(__py3_imports__)
 
 import sys
 from gevent.hub import get_hub, string_types, integer_types
+from gevent.hub import ConcurrentObjectUseError
 from gevent.timeout import Timeout
 
 is_windows = sys.platform == 'win32'
@@ -139,7 +140,8 @@ def wait(io, timeout=None, timeout_exc=_NONE):
         If you pass a value for this keyword, it is interpreted as for
         :class:`gevent.timeout.Timeout`.
     """
-    assert io.callback is None, 'This socket is already used by another greenlet: %r' % (io.callback, )
+    if io.callback is not None:
+        raise ConcurrentObjectUseError('This socket is already used by another greenlet: %r' % (io.callback, ))
     if timeout is not None:
         timeout_exc = timeout_exc if timeout_exc is not _NONE else _timeout_error('timed out')
         timeout = Timeout.start_new(timeout, timeout_exc)

gevent/builtins.py

@@ -43,6 +43,9 @@ def __import__(*args, **kwargs):
         # No such protection exists for monkey-patched builtins,
         # however, so this is necessary.
         args = args[1:]
+    # TODO: It would be nice not to have to acquire the locks
+    # if the module is already imported (in sys.modules), but the interpretation
+    # of the arguments is somewhat complex
     imp.acquire_lock()
     try:
         _g_import_lock.acquire()

gevent/event.py

@@ -3,6 +3,7 @@
 
 import sys
 from gevent.hub import get_hub, getcurrent, _NONE, PY3, reraise
+from gevent.hub import InvalidSwitchError
 from gevent.timeout import Timeout
 from gevent._tblib import dump_traceback, load_traceback
 from collections import deque
@@ -84,7 +85,8 @@ class Event(object):
             try:
                 try:
                     result = self.hub.switch()
-                    assert result is self, 'Invalid switch into Event.wait(): %r' % (result, )
+                    if result is not self:
+                        raise InvalidSwitchError('Invalid switch into Event.wait(): %r' % (result, ))
                 except Timeout as ex:
                     if ex is not timer:
                         raise
@@ -280,7 +282,8 @@ class AsyncResult(object):
             timer = Timeout.start_new(timeout)
             try:
                 result = self.hub.switch()
-                assert result is self, 'Invalid switch into AsyncResult.get(): %r' % (result, )
+                if result is not self:
+                    raise InvalidSwitchError('Invalid switch into AsyncResult.get(): %r' % (result, ))
             finally:
                 timer.cancel()
         except:
@@ -327,7 +330,8 @@ class AsyncResult(object):
             timer = Timeout.start_new(timeout)
             try:
                 result = self.hub.switch()
-                assert result is self, 'Invalid switch into AsyncResult.wait(): %r' % (result, )
+                if result is not self:
+                    raise InvalidSwitchError('Invalid switch into AsyncResult.wait(): %r' % (result, ))
             finally:
                 timer.cancel()
         except Timeout as exc:

gevent/greenlet.py

@@ -2,6 +2,7 @@
 
 import sys
 from gevent.hub import GreenletExit
+from gevent.hub import InvalidSwitchError
 from gevent.hub import PY3
 from gevent.hub import PYPY
 from gevent.hub import Waiter
@@ -448,7 +449,8 @@ class Greenlet(greenlet):
             t = Timeout.start_new(timeout)
             try:
                 result = self.parent.switch()
-                assert result is self, 'Invalid switch into Greenlet.get(): %r' % (result, )
+                if result is not self:
+                    raise InvalidSwitchError('Invalid switch into Greenlet.get(): %r' % (result, ))
             finally:
                 t.cancel()
         except:
@@ -478,7 +480,8 @@ class Greenlet(greenlet):
             t = Timeout.start_new(timeout)
             try:
                 result = self.parent.switch()
-                assert result is self, 'Invalid switch into Greenlet.join(): %r' % (result, )
+                if result is not self:
+                    raise InvalidSwitchError('Invalid switch into Greenlet.join(): %r' % (result, ))
             finally:
                 t.cancel()
         except Timeout as ex:

gevent/hub.py

@@ -65,6 +65,26 @@ get_ident = thread.get_ident
 MAIN_THREAD = get_ident()
 
 
+class LoopExit(Exception):
+    pass
+
+
+class BlockingSwitchOutError(AssertionError):
+    pass
+
+
+class InvalidSwitchError(AssertionError):
+    pass
+
+
+class ConcurrentObjectUseError(AssertionError):
+    # raised when an object is used (waited on) by two greenlets
+    # independently, meaning the object was entered into a blocking
+    # state by one greenlet and then another while still blocking in the
+    # first one
+    pass
+
+
 def spawn_raw(function, *args):
     """
     Create a new :class:`greenlet.greenlet` object and schedule it to run ``function(*args, **kwargs)``.
@@ -510,7 +530,7 @@ class Hub(greenlet):
         return greenlet.switch(self)
 
     def switch_out(self):
-        raise AssertionError('Impossible to call blocking function in the event loop callback')
+        raise BlockingSwitchOutError('Impossible to call blocking function in the event loop callback')
 
     def wait(self, watcher):
         """
@@ -529,7 +549,8 @@ class Hub(greenlet):
         watcher.start(waiter.switch, unique)
         try:
             result = waiter.get()
-            assert result is unique, 'Invalid switch into %s: %r (expected %r)' % (getcurrent(), result, unique)
+            if result is not unique:
+                raise InvalidSwitchError('Invalid switch into %s: %r (expected %r)' % (getcurrent(), result, unique))
         finally:
             watcher.stop()
 
@@ -649,10 +670,6 @@ class Hub(greenlet):
     threadpool = property(_get_threadpool, _set_threadpool, _del_threadpool)
 
 
-class LoopExit(Exception):
-    pass
-
-
 class Waiter(object):
     """
     A low level communication utility for greenlets.
@@ -768,7 +785,8 @@ class Waiter(object):
             else:
                 getcurrent().throw(*self._exception)
         else:
-            assert self.greenlet is None, 'This Waiter is already used by %r' % (self.greenlet, )
+            if self.greenlet is not None:
+                raise ConcurrentObjectUseError('This Waiter is already used by %r' % (self.greenlet, ))
             self.greenlet = getcurrent()
             try:
                 return self.hub.switch()

gevent/monkey.py

@@ -221,7 +221,8 @@ def patch_thread(threading=True, _threading_local=True, Event=False, logging=Tru
                 handler = wr() if callable(wr) else wr
                 if handler is None:
                     continue
-                assert hasattr(handler, 'lock'), "Unknown/unsupported handler %r" % handler
+                if not hasattr(handler, 'lock'):
+                    raise TypeError("Unknown/unsupported handler %r" % handler)
                 handler.lock = threading.RLock()
 
     if _threading_local:

gevent/queue.py

@@ -36,6 +36,7 @@ Empty = __queue__.Empty
 
 from gevent.timeout import Timeout
 from gevent.hub import get_hub, Waiter, getcurrent, PY3
+from gevent.hub import InvalidSwitchError
 
 
 __all__ = ['Queue', 'PriorityQueue', 'LifoQueue', 'JoinableQueue', 'Channel']
@@ -202,7 +203,8 @@ class Queue(object):
                 if self.getters:
                     self._schedule_unlock()
                 result = waiter.get()
-                assert result is waiter, "Invalid switch into Queue.put: %r" % (result, )
+                if result is not waiter:
+                    raise InvalidSwitchError("Invalid switch into Queue.put: %r" % (result, ))
             finally:
                 if timeout is not None:
                     timeout.cancel()
@@ -251,7 +253,8 @@ class Queue(object):
                 if self.putters:
                     self._schedule_unlock()
                 result = waiter.get()
-                assert result is waiter, 'Invalid switch into Queue.get: %r' % (result, )
+                if result is not waiter:
+                    raise InvalidSwitchError('Invalid switch into Queue.get: %r' % (result, ))
                 return self._get()
             finally:
                 if timeout is not None:
@@ -299,7 +302,8 @@ class Queue(object):
                 if self.putters:
                     self._schedule_unlock()
                 result = waiter.get()
-                assert result is waiter, 'Invalid switch into Queue.peek: %r' % (result, )
+                if result is not waiter:
+                    raise InvalidSwitchError('Invalid switch into Queue.peek: %r' % (result, ))
                 return self._peek()
             finally:
                 self.getters.discard(waiter)
@@ -520,7 +524,8 @@ class Channel(object):
             if self.getters:
                 self._schedule_unlock()
             result = waiter.get()
-            assert result is waiter, "Invalid switch into Channel.put: %r" % (result, )
+            if result is not waiter:
+                raise InvalidSwitchError("Invalid switch into Channel.put: %r" % (result, ))
         except:
             self._discard(item)
             raise

greentest/greentest.py

@@ -324,7 +324,7 @@ class TestCase(TestCaseMetaClass("NewBase", (BaseTestCase,), {})):
         if error is None:
             error = self.get_error()
         if type is not None:
-            assert error[1] is type, error
+            assert issubclass(error[1], type), error
         if value is not None:
             if isinstance(value, str):
                 assert str(error[2]) == value, error