nanomsg/nanomsg — pull request #164 by dbeck (2013-10-06 13:20:15+00:00)
Title: fixes for device tests and minor statemachin change in sinproc

(MIT License)

These changes fix device tests reported in #114

I found that it is possible (and often happens) that the thread in the main() function arrives to nn_send() before the socket has been bound in the deviceX() threads. There are a few potential solutions for this:

- make nn_connect() blocking and only continue when nn_bind() in the other thread has completed
- return error in nn_send() when it tries to send on a socket that the other side has not been boound. plus make sure the test code retries to send in this case
- or. my preferred solution was to treat this case as the library user's duty to make sure it doesn't try to send on an unbound socket. I implemented this by using nn_sem and make sure we don't send until bind happened.

After I fixed this, the original assert went away 'Unexpected action: state=2 source=27713 action=3 (src/transports/inproc/sinproc.c:273)', but the device test started to fail in another point:

'Unexpected source: state=4 source=27713 action=4 (src/transports/inproc/sinproc.c:352)'

This was a lot more subtle thing. When we send data on an inproc socket we put the data on the msgqueue of the receiver and enqueue an event into the receiver's state machine. In the device test the nn_send() is done by the nn_device() on a different thread and the nn_recv() happens on the main thread. In nn_recv() we gather the data from the msgqueue and process the fsm event. Furthermore we do an nn_close() in the main thread after nn_recv().

- Device Thr: nn_send()
- Device Thr: put data in the msgqueue
- Device Thr: enqueue NN_SINPROC_RECEIVED into the peer
- Device Thr: return nn_send()

- Main Thr: nn_recv()
- Main Thr: get data from the msgqueue
- Main Thr: checks fsm for pending events
- Main Thr: process pending events
- Main Thr: return nn_recv()
- Main Thr: nn_close()

There is a scenario where these overlap in an unfortunate way:

- Device Thr: nn_send()
- Main Thr: nn_recv()
- Device Thr: put data in the msgqueue
- Main Thr: get data from the msgqueue
- Main Thr: checks fsm for pending events
- Main Thr: process pending events (but there is nothing yet)
- Main Thr: return nn_recv()
- Main Thr: nn_colse()
- Device Thr: enqueue NN_SINPROC_RECEIVED into the peer (Thread1) ASSERT

The Device thread that tries to add the NN_SINPROC_RECEIVED event does assert() because the statemachine did't allow any events in the NN_SINPROC_STATE_DISCONNECTED state.

There are a few possible solutions to this issue:

- make sure nn_recv() doesn't return before it gets the NN_SINPROC_RECEIVED event. I voted against this because of the decreased concurrency and don't see any benefit at the same time.
- my preferred/implemented solution is to ignore NN_SINPROC_RECEIVED events in the NN_SINPROC_STATE_DISCONNECTED state because the only thing we would do in a normal case is clearing NN_SINPROC_FLAG_SENDING flag which is not a big loss on a disconnected socket.

Best regards, David