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

IO::Socket::Async.listen only tappable once. Binding address on tap inhibits use of Supply methods #2733

Closed
drclaw1394 opened this issue Mar 4, 2019 · 9 comments

Comments

Projects
None yet
3 participants
@drclaw1394
Copy link

commented Mar 4, 2019

The Problem

The supply returned from IO::Socket::Async.listen can only be tapped once without error. Calling methods that directly or indirectly tap subsequent times causes a 'address already in use' error.

  • Subsequent taps attempt to rebind the already in use address
  • Prevents distribution (multiple taps) of connected sockets (unless mirrored via another supply)
  • Cannot use the .wait method to wait until completion of the supply
  • Socket isn't actually listening when the .listen method is called, which is confusing and places the port/address in an 'unknown' state until supply is tapped
  • Conflates listening and tapping.

Expected Behavior

I would expect the socket to

  • start listening on the .listen call
  • immediately feed the returned supply with any incoming connections
  • any connections established prior to tapping of supply are lost, unless preserving supply is used.
  • Tapping the supply would provide the connected sockets only. No attempt to rebind address would be performed or necessary as it would be performed at the .listen method call.
  • Other supply methods such as .wait would work as described

Actual Behavior

The docs suggest that the IO::Socket::Async does not start to listen until the supply returned from .listen is tapped. Tapping a second time or using .wait (indirect tap) causes the 'address already in use' error.

Reproduction:

my $interface='0.0.0.0';
my $port=3344;
my $server = IO::Socket::Async.listen($interface, $port);
my $tap=$server.tap({
	.say;
});
$server.wait; # also a second $server.tap({;}); generates the same error

Output from this code with perl6 --ll-exception is :

address already in use
at SETTING::src/core/Exception.pm6:65 (/opt/local/share/perl6/runtime/CORE.setting.moarvm:rethrow)
from SETTING::src/core/Awaiter.pm6:24 (/opt/local/share/perl6/runtime/CORE.setting.moarvm:)
from SETTING::src/core/Awaiter.pm6:10 (/opt/local/share/perl6/runtime/CORE.setting.moarvm:await)
from SETTING::src/core/asyncops.pm6:33 (/opt/local/share/perl6/runtime/CORE.setting.moarvm:await)
from SETTING::src/core/asyncops.pm6:16 (/opt/local/share/perl6/runtime/CORE.setting.moarvm:await)
from SETTING::src/core/Supply.pm6:726 (/opt/local/share/perl6/runtime/CORE.setting.moarvm:wait)
from question.p6:8 (:)
from question.p6:1 (:)
from gen/moar/stage2/NQPHLL.nqp:1685 (/opt/local/share/nqp/lib/NQPHLL.moarvm:eval)
from gen/moar/stage2/NQPHLL.nqp:1928 (/opt/local/share/nqp/lib/NQPHLL.moarvm:evalfiles)
from gen/moar/stage2/NQPHLL.nqp:1851 (/opt/local/share/nqp/lib/NQPHLL.moarvm:command_eval)
from src/Perl6/Compiler.nqp:48 (/opt/local/share/nqp/lib/Perl6/Compiler.moarvm:command_eval)
from gen/moar/stage2/NQPHLL.nqp:1777 (/opt/local/share/nqp/lib/NQPHLL.moarvm:command_line)
from gen/moar/main.nqp:54 (/opt/local/share/perl6/runtime/perl6.moarvm:MAIN)
from gen/moar/main.nqp:42 (/opt/local/share/perl6/runtime/perl6.moarvm:)
from :1 (/opt/local/share/perl6/runtime/perl6.moarvm:

)
from :1 (/opt/local/share/perl6/runtime/perl6.moarvm:)

Operating system:

macOS 10.14.3
Darwin Kernel Version 18.2.0: Thu Dec 20 20:46:53 PST 2018; root:xnu-4903.241.1~1/RELEASE_X86_64

Compiler:

Rakudo version 2018.12 built on MoarVM version 2018.12

@Kaiepi

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2019

I knew about this behaviour but didn't consider adding it as part of my grant work on networking support. Depending on what other people think, I could make this work at some point within the next couple of months.

Edit:

  • Socket isn't actually listening when the .listen method is called, which is confusing and places the port/address in an 'unknown' state until supply is tapped

IO::Socket::Async sockets are created and bound as soon as .listen is called. The port/address just aren't accessible until the Supply is tapped. .listen will work differently in v6.e, though not in a way that'd address this.

@drclaw1394

This comment has been minimized.

Copy link
Author

commented Mar 5, 2019

Running socat as a server socat TCP-LISTEN:3344 - to prebind the port and then running the IO::Socket::Async.listen line above by itself doesn't throw an 'address in use error'. This would suggest the Socket is not bound at the .listen call?

Adding a single .tap or .wait on the Socket supply after the .listen in this situation does throw the error.

@Kaiepi

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2019

My bad, the socket's created and bound each time .tap is called. I was misremembering what the code looked like. Should've double-checked before posting

@Kaiepi

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2019

As for what you want from the expected behaviour, I think it's reasonable to expect, but one comment:

any connections established prior to tapping of supply are lost, unless preserving supply is used.

Servers should never drop connections for a reason like this. Using Supplier::Preserving would be necessary

@drclaw1394

This comment has been minimized.

Copy link
Author

commented Mar 7, 2019

Good point.

Perhaps the .listen method should basically call the system listen(). This will queue connections in the kernel until the system accept() is called.

On the first .tap method called, this eventually calls the system accept(), and starts dequeuing connections from the kernel, and feeding into the Supply.

This is opposed to .listen method immediately calling system accept() and then basically having to throw away or preserve the dequeued connections until the .tap method is called. I feel this conflates the intent of .listen and .tap and also is potentially double handling the connection queuing for no real gain.

@Kaiepi

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2019

Perhaps the .listen method should basically call the system listen(). This will queue connections in the kernel until the system accept() is called.

This isn't quite how it'd work with MoarVM since it uses libuv for async sockets. What could be done is have .listen call uv_tcp_bind, making it possible to get the host and port before calling .tap, then when .tap is called, call uv_listen and begin feeding the sockets accepted to the Supply. Likewise with the JVM, AsynchronousServerSocketChannel.bind could be called on .listen, then AsynchronousServerSocketChannel.accept on .tap. I have my doubts about whether this alone would make it possible to call .tap multiple times though. I have a feeling this would be rather complex to implement

@drclaw1394

This comment has been minimized.

Copy link
Author

commented Mar 8, 2019

I don't have much experience with libuv. Yes it sounds like it could be much trickier! I'm pretty new to rakudo / Perl6. Would you mind pointing me to the code that implements this?

I have even less experience with Java, but a quick websearch regarding AsynchronousServerSocketChannel comes up with:

If a thread initiates an accept operation before a previous accept operation has completed then an AcceptPendingException will be thrown.

Some sort or state management might need to be implemented then when tapping?
eg. if first tap then call accept. Otherwise ignore.

Or am I continuing to over simply the issue?

@Kaiepi

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2019

Would you mind pointing me to the code that implements this?

Rakudo: https://github.com/rakudo/rakudo/blob/master/src/core/IO/Socket/Async.pm6
MoarVM: https://github.com/MoarVM/MoarVM/blob/master/src/io/asyncsocket.c
nqp (JVM): https://github.com/perl6/nqp/blob/master/src/vm/jvm/runtime/org/perl6/nqp/io/AsyncServerSocketHandle.java

Some sort or state management might need to be implemented then when tapping?

I'm not sure. There would need to be some refactoring done so sockets received from the VM can be emitted to any taps that exist since the Supply will only ever emit to the first one created the way .listen is written, but state management might not be the solution to this.

@jnthn

This comment has been minimized.

Copy link
Member

commented Mar 11, 2019

The current behavior is correct, and consistent with the overall design of Supply and numerous other cases of Supply in Perl 6. Further, the problem stated (handling multiple concurrent connections) has an easy solution without introducing an inconsistency, and the proposed solution would break multiple pieces of existing functionality.

There are two kinds of Supply: live and on-demand. With a live Supply, one taps into an existing stream of events. The same events are delivered to all subscribers. Closing the tap does nothing except end the subscription, but doesn't inhibit the sender in sending further events. By contrast, tapping an on-demand Supply typically involves a resource acquisition or starting some process that leads to an asynchronous stream of values. Closing the tap releases the resource. Thus, it is entirely in line with the idea of an on-demand Supply that tapping it would start the listening, just as tapping a Supply.interval($n) starts a fresh timer.

Since it's permissible to pass 0 as the port and have a free port automatically selected, there already is a way in which tapping the same Supply returned from listen multiple times can lead to a useful result. The proposal would cause a regression to this behavior. The other issue is that the listening socket is closed by closing the Tap, and changing that would be a regression too.

While handling multiple concurrent connections is indeed a common requirement, that is easily handled:

sub handle-connection($conn) {
    supply {
        whenever $conn {
            $conn.write(some-costly-processing($_));
        }
    }
}
react whenever IO::Socket::Async.listen($host, $port) -> $conn {
    whenever handle-connection($conn) { }
}

It seems highly unlikely that the actual processing of new connections themselves would be a bottleneck.

Finally, wait seems to be misunderstood often enough that I do sometimes wonder if we might have been better dropping it prior to 6.c (it was introduced at the very start of the design process for async streams before we had all the things we ended up with, although I don't think we understood the ways it would be misunderstood back then :-)). Once one understands how the Supply model works, it's clear that the only way wait could possibly work is by doing its own tap call underneath, and it is correctly documented. However, those new to Perl 6 won't have a grasp on that yet, and so can have wrong expectations. Things like:

$supply.tap(&do-stuff);
$supply.wait;

Will almost never be what was intended; they work by accident but waste resources on a live Supply, and things will be worse with an on-demand Supply. The correct way if wanting to use wait is:

$supply.do(&do-stuff).wait;

Though for most folks it'd be better if they just learned this way:

react whenever $supply {
    do-stuff($_);
}

And the docs already show this way of doing things when dealing with async sockets, which hopefully means in the common case people will follow the structured programming approach rather than confuse themselves with the unstructured one. :-)

Since the behavior in Rakudo seems correct and the docs seem to show the right way already, I don't think there's much more to be done than to close this issue. It's somewhat tempting to open one pondering deprecation of .wait in a future Perl 6 version, though that'd not live here, but on the problem solving tracker, and I'm not quite convinced yet that the odd misunderstanding of it - when it's a de-emphasized feature - warrants that.

@jnthn jnthn closed this Mar 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.