Skip to content

Add .set_blocking to Socket and IO::FileDescriptor and deprecate the blocking args#15924

Closed
ysbaddaden wants to merge 25 commits intocrystal-lang:masterfrom
ysbaddaden:fix/docs-for-socket-and-file-fd-ownership+soft-deprecate-blocking-args
Closed

Add .set_blocking to Socket and IO::FileDescriptor and deprecate the blocking args#15924
ysbaddaden wants to merge 25 commits intocrystal-lang:masterfrom
ysbaddaden:fix/docs-for-socket-and-file-fd-ownership+soft-deprecate-blocking-args

Conversation

@ysbaddaden
Copy link
Collaborator

@ysbaddaden ysbaddaden commented Jun 24, 2025

Improves the documentation around the system file descriptors and handles in IO::FileDescriptor and Socket in relation to "adopting" the fd/handle into the IO system, or accessing it. I hope it starts to clarify the intent.

The documentation also soft deprecates the blocking argument of their constructors. Sadly, except for a few cases, I can't use the @[Deprecated] annotation and the usual two methods trick (one without the arg, and a deprecated one with the arg) because the blocking arg is positional and comes after default arguments. Now uses the @[Deprecated] annotation on methods args that depends on #15999.

Also deprecates the #blocking? and #blocking= methods that would mess with an IO::FileDescriptor or Socket blocking mode that has been configured by the event loop, that are replaced by class methods:

  • Socket.set_blocking(Bool) (UNIX and Windows targets);
  • IO::FileDescriptor.set_blocking(Bool) (UNIX targets only).

My doc-fu ain't the best. If you're more capable and find better phrasing, feel free to edit the PR 🙇

Follow up to #15804, #15754, #15753, #15750, #15930, #15977.

@ysbaddaden ysbaddaden self-assigned this Jun 24, 2025
@ysbaddaden ysbaddaden marked this pull request as ready for review June 24, 2025 17:22
@ysbaddaden ysbaddaden changed the title Fix/docs for socket and file fd ownership+soft deprecate blocking args Docs: fd/handle ownership Socket and IO::FileDescriptor + soft deprecate their blocking args Jun 24, 2025
@ysbaddaden ysbaddaden changed the title Docs: fd/handle ownership Socket and IO::FileDescriptor + soft deprecate their blocking args Docs: Socket and IO::FileDescriptor fd ownership & soft deprecate their blocking args Jun 24, 2025
@ysbaddaden
Copy link
Collaborator Author

Back to draft. Waiting for #15930.

@ysbaddaden ysbaddaden marked this pull request as draft June 26, 2025 16:17
@ysbaddaden ysbaddaden force-pushed the fix/docs-for-socket-and-file-fd-ownership+soft-deprecate-blocking-args branch from 3c44c1d to cc31269 Compare June 26, 2025 16:28
ysbaddaden added 13 commits July 3, 2025 10:53
Explicits that the fd/handle will be configured to match the event loop
requirements when adopted in the Socket types (its configuration will be
changed).

Explicits that `#fd` returns the fd/handle configured for the current
event loop, and no specific configuration must be expected.

Explicits that changing a socket's blocking mode can break the event
loop, and as such must be treated carefully.
Explicits that the fd/handle will be configured to match the event loop
requirements when adopted in the Socket types (its configuration will be
changed).

Explicits that `#fd` returns the fd/handle configured for the current
event loop, and no specific configuration must be expected.

Explicits that changing a socket's blocking mode can break the event
loop, and as such must be treated carefully.
@ysbaddaden ysbaddaden force-pushed the fix/docs-for-socket-and-file-fd-ownership+soft-deprecate-blocking-args branch from cc31269 to 27db4c0 Compare July 3, 2025 08:55
@ysbaddaden
Copy link
Collaborator Author

Rebased from master to remove conflicts with #15930.

@ysbaddaden
Copy link
Collaborator Author

ysbaddaden commented Jul 8, 2025

I realize I only added Socket.set_blocking(fd, blocking) but no equivalent for File or at least IO::FileDescriptor. Maybe we'd like to have it?

EDIT: I didn't write it because we can't change the mode on Windows. It might be useful on UNIX, though.

@ysbaddaden
Copy link
Collaborator Author

ysbaddaden commented Jul 13, 2025

I added IO::FileDescriptor.set_blocking and made it raise on Windows.

I added specs for it and Socket.set_blocking that fixes a few issues.

straight-shoota added a commit that referenced this pull request Jul 15, 2025
Extracted documentation-only changes from #15924

Co-authored-by: Julien Portalier <julien@portalier.com>
@straight-shoota
Copy link
Member

I extracted the doc enhancements to #15977 for 1.17.
We'll have to resolve conflicts (mostly minor typografic changes) and then it's ready for the merge window of 1.18.

@ysbaddaden ysbaddaden changed the title Docs: Socket and IO::FileDescriptor fd ownership & soft deprecate their blocking args Add .set_blocking to Socket and IO::FileDescriptor and deprecate the blocking args Jul 17, 2025
@ysbaddaden
Copy link
Collaborator Author

ysbaddaden commented Jul 17, 2025

I fixed the conflicts with master, but keep the PR into the "in progress" state.

Since we can annotate a method argument, and there's no rush to merge this PR, I'll try to see if I can get compiler warnings for a deprecated method argument (we can annotate them), and update this PR accordingly. We'll need docs to reflect this, too, but compilation warnings would be a good start.

@ysbaddaden
Copy link
Collaborator Author

Merged with master and updated to use the deprecated annotation on the blocking args directly (depends on #15999).

@straight-shoota
Copy link
Member

straight-shoota commented Jul 29, 2025

Actually, we should separate the addition of .set_blocking and the deprecations into two separate PRs. The former is a feature addition, the latter a deprecation. They should show up as separate changes.

Sorry for being that guy again. But I'm sure it's for the best 🙇

@ysbaddaden
Copy link
Collaborator Author

Argh, The Crystal 1.0 parser doesn't support annotations on method args. It was only added in #12044 for Crystal 1.5.

@ysbaddaden
Copy link
Collaborator Author

Split into #16033 and #16034.

@ysbaddaden ysbaddaden closed this Jul 29, 2025
@ysbaddaden ysbaddaden deleted the fix/docs-for-socket-and-file-fd-ownership+soft-deprecate-blocking-args branch July 29, 2025 15:41
ysbaddaden added a commit to ysbaddaden/crystal that referenced this pull request Aug 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants