Skip to content

Use Swift System to implement NonBlockingFileIO #2099

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

Conversation

simonjbeaumont
Copy link
Contributor

@simonjbeaumont simonjbeaumont commented May 6, 2022

Motivation:

#1957 suggests we make use of Swift System for the NonBlockingFileIO implementation which would also reduce its dependency on NIOPosix.

Modifications:

  • Add dependency on Swift System package.
  • Replace implementation of NonBlockingFileIO with System's FileDescriptor-based implementation.

To do:

  • Currently this PR depends on an unmerged PR in swift-system, this needs to be merged and then we need to depend on a released version.
  • Decide if we want to expose the withUnsafeSystemFileDescriptor function (probably not initially?)

@simonjbeaumont simonjbeaumont changed the title WIP: Use Swift System to implement NonBlockingFileIO Use Swift System to implement NonBlockingFileIO May 23, 2022
@simonjbeaumont simonjbeaumont marked this pull request as ready for review May 23, 2022 09:54
@simonjbeaumont
Copy link
Contributor Author

@Lukasa any thoughts on this one?

@@ -500,3 +499,11 @@ public struct NonBlockingFileIO {
}

}

extension NIOFileHandle {
public func withUnsafeSystemFileDescriptor<T>(_ body: (SystemPackage.FileDescriptor) throws -> T) throws -> T {
Copy link
Member

@weissi weissi Jun 1, 2022

Choose a reason for hiding this comment

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

@compnerd / @milseman Does swift-system have FileDescriptor on Windows and all other platforms?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does have FileDescriptor on Windows but, now you mention it, we explicitly opted to not include resize() on Windows. See apple/swift-system#82.

Copy link
Contributor

Choose a reason for hiding this comment

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

That is easily accomplished with SetFilePointerEx and SetEndOfFile; you will need _get_osfhandle to convert the file descriptor to the appropriate handle.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @compnerd. I guess that means that this (NIO) PR is probably blocked on a follow-up PR to swift-system to make sure that Windows has a FileDescriptor.resize(to:) implementation.

Copy link
Contributor Author

@simonjbeaumont simonjbeaumont Jun 7, 2022

Choose a reason for hiding this comment

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

Here's the PR for a Windows implementation of FileDescriptor.resize(to:): apple/swift-system#89

Copy link

@1proprogrammerchant 1proprogrammerchant left a comment

Choose a reason for hiding this comment

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

seems the commits are out of date to the branch

@simonjbeaumont
Copy link
Contributor Author

seems the commits are out of date to the branch

Yeah, this one died on the vine. And the addition of ftruncate to swift-system for Windows got reverted because it broke the build and we don't have any CI on swift-system for Windows so I'm a bit hesitant to add it again.

@Lukasa
Copy link
Contributor

Lukasa commented Jan 14, 2025

We’ve deprecated NonBlockingFileIO.

@Lukasa Lukasa closed this Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants