Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Received messages via push/pull are empty #25

Closed
tobi-tobsen opened this issue Nov 11, 2012 · 7 comments
Closed

Received messages via push/pull are empty #25

tobi-tobsen opened this issue Nov 11, 2012 · 7 comments

Comments

@tobi-tobsen
Copy link
Contributor

Bug

Received messages via push/pull are empty. Both the Size property as well as the length of the Data byte[] property are 0.

Steps to reproduce

  • get the performance test from Perf throughput #24
  • compile the solution
  • run the following: local_thr.exe tcp://127.0.0.1:4711 300 1000
  • run the following: remote_thr.exe tcp://127.0.0.1:4711 300 1000
@somdoron
Copy link
Member

i will fix it, it better to fix this to have same behavior as c++.

@tobi-tobsen
Copy link
Contributor Author

I tried removing the the call to close() and it works now. #27

I am not sure whats the best behaviour/API is:

  • If there is a Close() should one call it after the message has been send?
  • A Message being IDisposable has the same semantics I guess
  • socket.Send(Msg) seems to fiddle around with the message after the call. In order to work correctly with Close() a copy might have to be made somewhere. I don't think that is a good thing to do either. I am currently not sure how the c++ implementation handles that.

Maybe garbage collection is sufficient and there is no need for an explicit call to close?

@somdoron
Copy link
Member

c++ is copy the msg class but the buffer is not copied. we may do the same

@somdoron
Copy link
Member

i just added a new pull request with performance feature from original zeromq.

the throughput performance is looking good compare to the original.

i hope latency check is coming soon

@tobi-tobsen
Copy link
Contributor Author

From inspection of your pullrequest, the issue seems to be fixed and it seems that a call to Close is still not necessary but doesn't hurt either since it is just setting the message type to VSM (very small message) (https://github.com/zeromq/netmq/blob/master/src/NetMQ/zmq/Msg.cs#L273).

If you want so, I'll add the latency performance test after work.

@somdoron
Copy link
Member

that will be great. thanks

jgoz added a commit that referenced this issue Nov 12, 2012
fixed issue #25, fixed issue #26, fixed collection was modified and msg close
@somdoron
Copy link
Member

fixed in last merge pull

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

No branches or pull requests

2 participants