Skip to content

Commit

Permalink
Deprecate the TCP.Engine.pending_reads method.
Browse files Browse the repository at this point in the history
There's no need for this method anymore.
Loading pending reads into the read stream will now be handled
automatically by the TCP engine, which will yield an `IO.Action.Read`
action for each time a pending read is completed.

So you can just remove this function call from your code and de-indent
the body of your yield block, such that it becomes part of the outer code.
If you need to know the number of bytes available, then you can call
`io.read_stream.bytes_ahead_of_marker` to find out.

For now, calling it will do nothing but yield once with the value
of `io.read_stream.bytes_ahead_of_marker` to maintain compatibility.
But the method will be removed soon.

---

There's no reason why the library user needs to be the one to tell
the engine to handle pending reads - it was a bad design from the start,
but it also turns out to be a blocker for Windows support,
wherein pending reads will work a bit differently.
Also it was inconsistent with the `StdIn` library which doesn't
have this method in its engine, so that's also cleared up now.
  • Loading branch information
jemc committed Feb 15, 2023
1 parent 192ace1 commit d30c3ca
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 25 deletions.
24 changes: 11 additions & 13 deletions spec/TCP.Spec.savi
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,14 @@
|
@env.err.print("[Echoer] Failed to get local and/or remote address")
)

| IO.Action.Read |
@io.pending_reads -> (bytes_available |
bytes val = @io.read_stream.extract_all
@env.err.print("[Echoer] Received: \(Inspect[bytes])")
@io.write_stream << bytes
try @io.flush! // TODO: should we flush automatically on close below?
@io.close
)
bytes val = @io.read_stream.extract_all
@env.err.print("[Echoer] Received: \(Inspect[bytes])")
@io.write_stream << bytes
try @io.flush! // TODO: should we flush automatically on close below?
@io.close

| IO.Action.Closed |
@env.err.print("[Echoer] Closed")
@listener.dispose // ask the listener to close too
Expand Down Expand Up @@ -105,12 +105,10 @@
@env.err.print(@io.connect_error.name)

| IO.Action.Read |
@io.pending_reads -> (bytes_available |
if (bytes_available >= b"Hello, World!".size) (
bytes val = @io.read_stream.extract_all
@env.err.print("[EchoClient] Received: \(Inspect[bytes])")
@io.close
)
if (@io.read_stream.bytes_ahead_of_marker >= b"Hello, World!".size) (
bytes val = @io.read_stream.extract_all
@env.err.print("[EchoClient] Received: \(Inspect[bytes])")
@io.close
)

| IO.Action.Closed |
Expand Down
39 changes: 27 additions & 12 deletions src/TCP.Engine.savi
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,21 @@
// This allows the listener to keep an accurate count of how many
// connections have been opened so far.
try @_listener.as!(IO.Actor(IO.Action)).io_deferred_action(IO.Action.ClosedChild)

// TODO: windows complete writes, flush-after-mute (pending writes logic from Pony)
// | IO.Action.Write |
// ...
yield action

| IO.Action.Read |
if Platform.is_windows (
None // TODO: @_windows_complete_reads(arg)
|
@_pending_reads_unix -> (yield action)
)

// TODO: windows complete writes, flush-after-mute (pending writes logic from Pony)
// | IO.Action.Write |
// ...
|
yield action
)
yield action
)
@

Expand All @@ -67,21 +76,27 @@
:fun ref flush!
@write_stream.flush!

// TODO: Remove this deprecated method.
:: DEPRECATED: There's no need for this method anymore.
:: Loading pending reads into the read stream will now be handled
:: automatically by the TCP engine, which will yield an `IO.Action.Read`
:: action for each time a pending read is completed.
::
:: So you can just remove this function call from your code and de-indent
:: the body of your yield block, such that it becomes part of the outer code.
:: If you need to know the number of bytes available, then you can call
:: `io.read_stream.bytes_ahead_of_marker` to find out.
:fun ref pending_reads
:yields USize for None
if Platform.is_windows (
None // TODO: @_windows_complete_reads(arg)
|
@_pending_reads_unix -> (bytes_available | yield bytes_available)
)
yield @read_stream.bytes_ahead_of_marker
@

:fun ref _pending_reads_unix None
:yields USize for None
:yields None for None
while @io.is_readable (
try (
bytes_read = @read_stream.receive_from!(@io)
if (bytes_read > 0) (yield @read_stream.bytes_ahead_of_marker)
if (bytes_read > 0) (yield None)
)
)

Expand Down

0 comments on commit d30c3ca

Please sign in to comment.