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.

Sign up for GitHub

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

RFC 0007: Event loop refactor #7

Merged
merged 27 commits into from
Nov 15, 2024
Merged

RFC 0007: Event loop refactor #7

Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift click to select a range
9a93501
Event loop refactor
straight-shoota May 2, 2024
04a7c37
Fix filename
straight-shoota May 7, 2024
c70c896
Fill guide-level explanation
straight-shoota May 7, 2024
ba1e1a4
Add abstract interface description
straight-shoota May 7, 2024
3e40217
Fix associate *Socket* subsection with *Optional event loop features*
straight-shoota May 27, 2024
3bff4d1
fixup
straight-shoota May 27, 2024
9afc76f
Resolve *`read` and `write` behaviour*
straight-shoota May 28, 2024
9ab3a8e
Resolve *Timeout and Resume events*
straight-shoota May 28, 2024
0647def
Fix typo
straight-shoota May 28, 2024
113e401
Drop `EventLoop#send`, `#receive`
straight-shoota May 28, 2024
b1978fa
Separate `EventLoop` interface into modules
straight-shoota May 28, 2024
a42930c
typo
straight-shoota May 28, 2024
5db0edb
Add `run` and `interrupt` for completeness
straight-shoota May 28, 2024
a08f935
fixup
straight-shoota May 29, 2024
5b7f663
Improve API docs for `EventLoop::Socket`
straight-shoota May 29, 2024
8b9b09b
Update text/0007-event_loop-refactor.md
straight-shoota May 30, 2024
04462e8
Apply suggestions from code review
straight-shoota May 31, 2024
8eee22e
Update implemented API
straight-shoota Nov 12, 2024
6334e16
Resolve `Socket` question
straight-shoota Nov 12, 2024
526b3d8
cleanup
straight-shoota Nov 12, 2024
53a1861
Add reference to RFC 0009
straight-shoota Nov 12, 2024
1061a51
Fix `write`/`read` docs
straight-shoota Nov 14, 2024
246c87f
Drop unresolved questions
straight-shoota Nov 14, 2024
f74db24
Turn _Type for sizes_ into a future possibility
straight-shoota Nov 14, 2024
70942fa
Turn more open questions into future possibilities
straight-shoota Nov 14, 2024
b7da5cc
Drop *Rationale and alternatives*
straight-shoota Nov 14, 2024
252f546
Add *Rationale and alternatives*
straight-shoota Nov 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add abstract interface description
  • Loading branch information
@straight-shoota
straight-shoota committed May 7, 2024
commit ba1e1a4241122e6a786e844a40aa42ab0f01d0c1
60 changes: 59 additions & 1 deletion text/0007-event_loop-refactor.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 72,65 @@ Further extensions can widen the scope of the event loop.

# Reference-level explanation

TBD
The new `EventLoop` defines the following interface for issuing operations on the event loop:

```cr
module EventLoop
beta-ziliani marked this conversation as resolved.
Show resolved Hide resolved
# Reads at least one byte from the file descriptor into *slice* and continues
# fiber when the read is complete.
# Returns the number of bytes read.
abstract def read(file : Crystal::System::FileDescriptor, slice : Bytes) : Int32

# Reads at least one byte from the socket into *slice* and continues fiber
# when the read is complete.
# Returns the number of bytes read.
abstract def read(socket : ::Socket, slice : Bytes) : Int32

# Writes at least one byte from *slice* to the file descriptor and continues
# fiber when the write is complete.
# Returns the number of bytes written.
abstract def write(file : Crystal::System::FileDescriptor, slice : Bytes) : Int32

# Writes at least one byte from *slice* to the socket and continues fiber
# when the write is complete.
# Returns the number of bytes written.
abstract def write(file : ::Socket, slice : Bytes) : Int32

# Accepts an incoming TCP connection on the socket and continues fiber when a
# connection is available.
# Returns a handle to the socket for the new connection.
abstract def accept(socket : ::Socket) : ::Socket::Handle?

# Opens a connection on *socket* to the target *address* and continues fiber
# when the connection has been established.
# Returns `IO::Error` but does not raise.
abstract def connect(socket : ::Socket, address : ::Socket::Addrinfo | ::Socket::Address, timeout : ::Time::Span?) : IO::Error?
Copy link
Member Author

Choose a reason for hiding this comment

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

The address type could perhaps be abstracted. We need to support both types, though they both boil down to the same interface: #to_unsafe returning a pointer to a LibC::Sockaddr and #size returning its size in bytes.

So it could be an option to define an interface for that and use that as type restriction. But we cannot actually make it strictly type-safe because the return type of #to_unsafe is Void* and we cannot enforce a pointer to LibC::Sockaddr (so Array would implement this interface for example).

Alternatively, we could remove the type restriction entirely because any type that fulfills the implicit interface should work.

I think it's better to be explicit, either with ::Socket::Addrinfo | ::Socket::Address or an abstract module interface.
Introducing a new model would also solve the issue that the Socket name space is not in core lib (see https://github.com/crystal-lang/rfcs/blob/rfc/0005/text/0007-event_loop-refactor.md#socket).

Copy link
Member Author

Choose a reason for hiding this comment

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

It's actually hard to stub out ::Socket::Addrinfo and ::Socket::Address because that would require defining the Socket namespace, which is a class that inherits IO and that's so much complexity we could just include socket.cr.

I've considered introducing a module which could be included in both types. But that's actually not a great solution for when we return an address.
So I think it's probably best to introduce an internal struct type which holds a reference to LibC::Sockaddr and its size.

Copy link
Member

Choose a reason for hiding this comment

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

So I think it's probably best to introduce an internal struct type which holds a reference to LibC::Sockaddr and its size.

Or to leave it as the union?

Copy link
Member Author

Choose a reason for hiding this comment

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

Those types are not in core lib. Their existence depends on require "socket". So leaving the union won't work as is. Trying to see if we can stub it out (or replace with an internal type) is one possible way to deal with that.

But I think a better approach is described in #7 (comment)


# Writes at least one byte from *slice* to the socket and continues fiber when
# the write is complete.
# Returns the number of bytes written.
abstract def send(socket : ::Socket, slice : Bytes) : Int32

# Writes at least one byte from *slice* to the socket with a target *address* (UDP)
# and continues fiber when the write is complete.
# Returns the number of bytes written.
abstract def send_to(socket : ::Socket, slice : Bytes, address : ::Socket::Address) : Int32

# Receives on the socket into *slice* and continues fiber when the package is
# completed
# Returns the number of bytes received.
abstract def receive(socket : ::Socket, slice : Bytes) : Int32
straight-shoota marked this conversation as resolved.
Show resolved Hide resolved

# Receives on the socket into *slice* and continues fiber when the package is
# completed.
# Returns a tuple containing the number of bytes received and the source address
# of the packet (UDP).
abstract def receive_from(socket : ::Socket, slice : Bytes) : Tuple(Int32, ::Socket::Address)

# Closes the *resource*.
abstract def close(resource) : Nil
end
```

# Drawbacks

Expand Down