[zeromq-dev] [PATCH] Improved response to socket violations

[Martin Lucina]

Pieter,

ph at imatix.com said:
> Attached is a patch I'd like to apply to 2.x to deal with socket
> violations in a more useful fashion.
> 
> It replaces the "Assertion failed: nbytes == sizeof (command_t)
> (mailbox.cpp:245)" message with an explicit message about not sharing
> sockets between threads.

Please, no! Arguments below so that people can see the actual change.

--- a/src/mailbox.cpp
+++ b/src/mailbox.cpp
@@ -219,7 +219,7 @@ int zmq::mailbox_t::recv (command_t *cmd_, bool block_)
     //  it to caller.
     int err = 0;
     ssize_t nbytes = ::recv (r, cmd_, sizeof (command_t), 0);
-    if (nbytes == -1 && (errno == EAGAIN || errno == EINTR))  
+    if (nbytes == -1 && (errno == EAGAIN || errno == EINTR))
         err = errno;
 
     //  Re-set the reader to non-blocking mode.
@@ -242,8 +242,11 @@ int zmq::mailbox_t::recv (command_t *cmd_, bool block_)
     errno_assert (nbytes != -1);
 
     //  Check whether we haven't got half of command.
-    zmq_assert (nbytes == sizeof (command_t));
-
+    if (nbytes != sizeof (command_t)) {
+        fprintf (stderr, "E: Likely attempt to use a socket from multiple threads\n");
+        fprintf (stderr, "I: Sockets are not threadsafe. Exiting application now.\n");
+        zmq_assert (0);
+    }
     return 0;
 }

Arguments against in rough order of seriousness:

1) The message itself is misleading. There are many people who run into
issues with mailbox due to a completely *different* problem, namely system
resource limits (e.g. OSX). This has nothing to do with socket migration.

2) The message is not canonical. There may be multiple contexts in use;
this does not tell the caller which context or call path caused the problem
so they'll probably need to fire up a debugger anyway to get that
information.

3) Last but not least, IMHO libraries should *not* print "helpful"
messages.  This leads to horrible practices, for example start a random Gtk
application; you will more often than not see all sorts of assertion
failures and other crap printed and it's obvious that no one cares, much
less does anything about it.

I realise you have an assert(0) there so the code *will* fail, but I'd
still like to avoid going down this path.

If we have a problem with users using sockets from multiple threads then
the documentation needs to stress that they not do that. If they do it
anyway, too bad for them!

-mato