Switch handler locking off by default

Salmon should have been thread-safe from the start.

salmon/handlers/queue.py

@@ -8,16 +8,14 @@
 import logging
 
 from salmon import handlers, queue
-from salmon.routing import nolocking, route_like, stateless
+from salmon.routing import route_like, stateless
 
 
 @route_like(handlers.log.START)
 @stateless
-@nolocking
 def START(message, to=None, host=None):
     """
     @stateless and routes however handlers.log.START routes (everything).
-    Has @nolocking, but that's alright since it's just writing to a Maildir.
     """
     logging.debug("MESSAGE to %s@%s added to queue.", to, host)
     q = queue.Queue('run/queue')

salmon/routing.py

@@ -36,8 +36,7 @@
 * @route_like -- Says that this function routes like another one.
 * @stateless -- Indicates this function always runs on each route encountered, and
   no state is maintained.
-* @nolocking -- Use this if you want this handler to run parallel without any
-  locking around Salmon internals.  SUPER DANGEROUS, add @stateless as well.
+* @locking -- Use this if you want this handler to be run one call at a time.
 * @state_key_generator -- Used on a function that knows how to make your state
   keys for the module, for example if module_name + message.To is needed to maintain
   state.
@@ -55,6 +54,7 @@
 import shelve
 import sys
 import threading
+import warnings
 
 ROUTE_FIRST_STATE = 'START'
 LOG = logging.getLogger("routing")
@@ -190,22 +190,11 @@ class RoutingBase:
 
     in your settings module.
 
-    RoutingBase does locking on every write to its internal data (which usually
-    only happens during booting and reloading while debugging), and when each
-    handler's state function is called.  ALL threads will go through this lock,
-    but only as each state is run, so you won't have a situation where the chain
-    of state functions will block all the others.  This means that while your
-    handler runs nothing will be running, but you have not guarantees about
-    the order of each state function.
-
-    However, this can kill the performance of some kinds of state functions,
-    so if you find the need to not have locking, then use the @nolocking
-    decorator and the Router will NOT lock when that function is called.  That
-    means while your @nolocking state function is running at least one other
-    thread (more if the next ones happen to be @nolocking) could also be
-    running.
-
-    It's your job to keep things straight if you do that.
+    RoutingBase assumes that both your STATE_STORE and handlers are
+    thread-safe. For handlers that cannot be made thread-safe, use @locking and
+    RoutingBase will use locks to make sure that handler is only called one
+    call at a time. Please note that this will have a negative impact on
+    performance.
 
     NOTE: See @state_key_generator for a way to change what the key is to
     STATE_STORE for different state control options.
@@ -351,11 +340,11 @@ def deliver(self, message):
         for func, matchkw in self._collect_matches(message):
             LOG.debug("Matched %r against %s.", message.To, func.__name__)
 
-            if salmon_setting(func, 'nolocking'):
-                self.call_safely(func, message,  matchkw)
-            else:
+            if salmon_setting(func, 'locking'):
                 with self.call_lock:
                     self.call_safely(func, message, matchkw)
+            else:
+                self.call_safely(func, message,  matchkw)
 
             called_count += 1
 
@@ -577,19 +566,21 @@ def stateless(func):
 
 def nolocking(func):
     """
-    Normally salmon.routing.Router has a lock around each call to all handlers
-    to prevent them from stepping on each other.  It's assumed that 95% of the
-    time this is what you want, so it's the default.  You probably want
-    everything to go in order and not step on other things going off from other
-    threads in the system.
-
-    However, sometimes you know better what you are doing and this is where
-    @nolocking comes in.  Put this decorator on your state functions that you
-    don't care about threading issues or that you have found a need to
-    manually tune, and it will run it without any locks.
+    Does nothing, as no locking is the default now
+    """
+    warnings.warn("@nolocking is redundant and can be safely removed from your handler %s" % func,
+                  category=DeprecationWarning, stacklevel=2)
+    return func
+
+
+def locking(func):
+    """
+    Salmon assumes your handlers are thread-safe, but is not always the case.
+    Put this decorator on any state functions that are not thread-safe for
+    whatever reason.
     """
     attach_salmon_settings(func)
-    func._salmon_settings['nolocking'] = True
+    func._salmon_settings['locking'] = True
     return func
 
 

salmon/server.py

@@ -299,8 +299,7 @@ def __init__(self, queue_dir, sleep=10, size_limit=0, oversize_dir=None, workers
         """
         The router should be fully configured and ready to work, the queue_dir
         can be a fully qualified path or relative. The option workers dictates
-        how many threads are started to process messages. Consider adding
-        ``@nolocking`` to your handlers if you are able to.
+        how many threads are started to process messages.
         """
         self.queue = queue.Queue(queue_dir, pop_limit=size_limit,
                                  oversize_dir=oversize_dir)

tests/data/test_app/test_handlers/dump.py

@@ -1,11 +1,10 @@
 from salmon import queue
-from salmon.routing import nolocking, route, stateless
+from salmon.routing import route, stateless
 from salmon.utils import settings
 
 
 @route("(to)@(host)", to=".+", host="example.com")
 @stateless
-@nolocking
 def START(message, to=None, host=None):
     inbox = queue.Queue(settings.QUEUE_PATH)
     inbox.push(message)

tests/handlers/simple_fsm_mod.py

@@ -1,4 +1,4 @@
-from salmon.routing import Router, nolocking, route, route_like, state_key_generator, stateless
+from salmon.routing import Router, route, route_like, state_key_generator, stateless
 
 
 @state_key_generator
@@ -47,7 +47,6 @@ def END(message, anything=None, host=None):
 
 @route(".*")
 @stateless
-@nolocking
 def PASSING(message, *args, **kw):
     print("PASSING", args, kw)