Skip to content
This repository has been archived by the owner on Apr 24, 2020. It is now read-only.

Add Multipart Message Support #75

Merged
merged 3 commits into from Jan 26, 2012
Merged

Add Multipart Message Support #75

merged 3 commits into from Jan 26, 2012

Conversation

joshrtay
Copy link
Contributor

@joshrtay joshrtay commented Jan 5, 2012

Added Socket#send_mutli and fixed bug in Socket#_flush that occured when a message was received in the middle of send_multi call.

@tj
Copy link
Collaborator

tj commented Jan 5, 2012

could you add a test? I'm not really sold on having a second method for it, plus this is camelcase land :p but yeah we could add variable arg support back to send() perhaps, or just the flag with .send()

@joshrtay
Copy link
Contributor Author

joshrtay commented Jan 5, 2012

what do you think?

this._outgoing.push([msg, flags || 0]);
Socket.prototype.send = function(msgs, flags) {
if (!Array.isArray(msgs)) msgs = [msgs]

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method is hot so we should make sure to optimize, things like ditching msg.forEach( etc

@joshrtay
Copy link
Contributor Author

joshrtay commented Jan 6, 2012

In the current code base multipart send is buggy, so you should probably either implement the emit on nextTick change or the don't flush if sndmore flag change.

The problem is that if you call send('1',zmq.ZMQ_SNDMORE) and then send('2'), the flush from first send may emit a 'message' event and if one of the listeners of the message event calls send then the frames of message are not going to be delivered in the expected order.

@ronkorving
Copy link
Collaborator

+1 on fixing this issue. ZeroMQ without multipart is unusable (for me, anyway).

@vvo
Copy link

vvo commented Jan 26, 2012

Hello, indeed before near complete rewrite of zmq node we were able to do this :

sock.send('frame1', 'frame2', 'frame3');

and then receive

sock.on('message', function(msgArr) {
console.log(msgArr[0], msgArr[1], msgArr[2]);
});

Breaking this made the module not usable by people using this kind of messages.

+1 so !

tj added a commit that referenced this pull request Jan 26, 2012
Add Multipart Message Support
@tj tj merged commit b319dec into JustinTulloss:master Jan 26, 2012
@tj
Copy link
Collaborator

tj commented Jan 26, 2012

sorry for the delay :D

@ronkorving
Copy link
Collaborator

Thanks, much appreciated!

@joshrtay
Copy link
Contributor Author

ya - thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants