Updates and fixes for Akka.IO UDP support #3005
Conversation
|
@Horusiath some UDP specs on Mono failing following this change |
|
Because you also refactored to use new patterns and stuff. Its hard to review. Could you point out what you changed to fix the problem ? |
|
Essentially the problem was that I didn't updated the UDP handler code in line to the changes I was making in
I've extracted logic responsible for recognizing which method to use ( I'll take a look at issue on Mono later today. |
|
@Horusiath you made a lot of API changes, it will be impossible to ship it in 1.3.x. Is it possible to bring back some constructors and properties and make them obsoleted? |
|
@alexvaluyskiy 90% of API changes is about sealing message classes (which should be sealed). There's a one change in the I guess it's not a big deal, as it's more to be used by us than users, and we can ask for forgiveness the 1-2 people, whos project will broke by that. |
364a835 to
d4012aa
Compare
|
The reason of failure on Mono found: Mono doesn't support using BufferList at this point. This probably means, that the same problem concerns TCP, we just didn't have tests to detect that. As I definitely don't want to fallback the whole ability to send multiple buffers at once or build a separate (and very complex) buffer send awaiting logic just to cover Mono, my proposal is: If we detect, that we're on Mono, we use Are we fine with that? /cc @akkadotnet/core |
| // Mono doesn't support BufferList - falback to compacting ByteString | ||
| var compacted = data.Compact(); | ||
| var buffer = compacted.Buffers[0]; | ||
| args.SetBuffer(buffer.Array, buffer.Offset, buffer.Count); |
There was a problem hiding this comment.
If we detect a Mono runtime, we'll try to fallback and compact multiple byte buffers into one (this operations copies buffers onto newly allocated array).
Fine with me. Not our job to implement the missing parts of the Mono runtime. |
| @@ -61,6 +62,20 @@ public void The_UDP_Fire_and_Forget_implementation_must_be_able_to_send_without_ | |||
| } | |||
|
|
|||
| [Fact] | |||
| public void The_UDP_Fire_and_Forget_implementation_must_be_able_to_send_multipart_ByteString_without_binding() | |||
| // Mono doesn't support BufferList - falback to compacting ByteString | ||
| var compacted = data.Compact(); | ||
| var buffer = compacted.Buffers[0]; | ||
| args.SetBuffer(buffer.Array, buffer.Offset, buffer.Count); |
| @@ -152,44 +152,48 @@ public NoAck(object token) | |||
| /// </summary> | |||
| public sealed class Send : Command | |||
| { | |||
| [Obsolete("Akka.IO.Udp.Send public constructors are obsolete. Use `Send.Create` or `Send(ByteString, EndPoint, Event)` instead.")] | |||
| public Send(IEnumerator<ByteBuffer> payload, Event ack) | |||
| @@ -35,6 +35,16 @@ public sealed class ByteString : IEquatable<ByteString>, IEnumerable<byte> | |||
| #region creation methods | |||
|
|
|||
| /// <summary> | |||
| /// INTERNAL API: remove this method once <see cref="Udp.Send"/> public constructor will be removed. | |||
| /// </summary> | |||
| internal static ByteString FromBuffers(IEnumerator<ByteBuffer> buffers) | |||
This should fix the issue reported in #2995 (PR contains tests which reproduced the issue). It also upgrades the codebase to C# 7.