zeromq.node icon indicating copy to clipboard operation
zeromq.node copied to clipboard

Socket.pause() only pauses emitting of 'message' events, not actual retrieval of messages

Open elyscape opened this issue 8 years ago • 7 comments

When calling pause() on a pull socket, messages continue to be retrieved in the background. The only difference is that no 'message' events are fired until resume() is called on the socket, at which point events are emitted for all previously-retrieved messages. This is unexpected behavior and should probably either be documented or fixed to behave as expected.

elyscape avatar Dec 11 '15 21:12 elyscape

Unfortunately I think you're right, I had a feeling that might be the case :(

reqshark avatar Dec 11 '15 23:12 reqshark

here just testing an emit() or read() call at the JS layer, not the actual socket library's recv.

  it('should not emit messages after pause()', function(done){
      var push = zmq.socket('push')
        , pull = zmq.socket('pull');

      var n = 0;

      pull.on('message', function(msg){
        if(n++ === 0) {
          msg.toString().should.equal('foo');
        }
        else{
          should.not.exist(msg);
        }
      });

https://github.com/JustinTulloss/zeromq.node/blob/master/test/socket.push-pull.js#L42-L73

also wtf is should.not.exist(msg);? Undefined? Null? exist is not a type, IMO nor a convention in JS

reqshark avatar Dec 11 '15 23:12 reqshark

I think should.not.exist is part of the mocking framework...

kroeders avatar Dec 15 '15 02:12 kroeders

Could we have unref() / ref() APIs just like the intrinsic Net.Socket objects? I think we could implement that by passing poll_handle_ of binding.cc to uv_unref() and uv_ref() functions.

achimnol avatar Mar 29 '16 16:03 achimnol

Could we have unref() / ref() APIs just like the intrinsic Net.Socket objects?

sounds interesting, what's the use case?

reqshark avatar Mar 29 '16 19:03 reqshark

There may be cases that we want to skip checking pending RX/TX data. Just like the vanilla node sockets, one would want terminate the main program without explicitly closing sockets. I think this is particularly useful for ZMQ since it provides "asynchronous" semantics by its design, such as PUB/SUB sockets where the "actual" OS-level socket connection does not matter.

For me, I'm implementing an event-loop waiter as a C++ addon to nodejs that repeatedly calls uv_run() until there is no more pending events to execute and wait until arbitrary user code finishes, where I do not have any prior information on what callbacks the input code would generate. (This is what exactly the main loop of nodejs itself does to decide if it should terminate or not, for the entire program. Now I want to use the same blocking mechanism for just a specific region of codes.) To make this work, I need to temporarily "unref" all existing callbacks such as timers and sockets.

BTW, my feature request may not be aligned with this thread, though I have crashed here after trying pause() for my purpose. So I've created a new issue: #502.

achimnol avatar Mar 30 '16 05:03 achimnol

I've found another bug concerning pause/resume. Socket.pause() only pauses AFTER flushing messages queued in the binding. User however might call pause() within 'message' handler but it won't stop flushing messages until the internal binding's queue is empty.

I think this line: https://github.com/JustinTulloss/zeromq.node/blob/7e2d99b33864fb54acec01b19542805257ffafa1/lib/index.js#L640 should be replaced with:

return !this._paused;

Thanks to this another _flushRead() will NOT be called from _flushReads() until user calls resume();

royaltm avatar Oct 28 '16 21:10 royaltm