Remove not on eventLoop precondition for NIOPipeBootstrap #1977
Conversation
Motivation: Currently you cannot create a NIOPipeBootstrap when you are on an event loop due to a precondition check. This check seeks to prevent consumers of the API from passing in a file descriptor thats referencing a file on disk or on the network. The method at hand 'validateFileDescriptorIsNotAFile' uses fstat, which potentially could block the event loop especially if the fd is referencing a file over the network. This check however prevents certain use cases of SwiftNIO, where the downsides of a consumer of this api blocking their own event loop do not weigh in against preventing an entire use case from SwiftNIO. Modifications: Removed the precondition Result: After this change it will be possible to feed file descriptors into the bootstrap which potentially block the current event loop. A potential (portable) replacement for fstat still has to be found in order to solve this problem completely.
|
Can one of the admins verify this patch? |
11 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
@swift-nio-bot test this please |
|
@jabwd Thanks for this patch! Ideally we'd write a test for this to confirm that it works: can you replicate a simple one of the NIOPipeChannel tests and run the bootstrap from inside an |
|
@Lukasa I created a simple test; I'm not super sure if the failure modes are acceptable however. If I reintroduce the precondition It'll essentially crash the test rather than just count as a fail. |
jabwd
force-pushed
the
remove-pipebootstrap-precondition
branch
from
5e24227
to
2ccfdb7
Compare
|
@swift-nio-bot test this please |
|
Do you mind running |
|
My bad, forgot about running that! Changes have been pushed 👍 |
|
@swift-nio-bot test this please |
Remove not on eventLoop precondition for NIOPipeBootstrap
Motivation:
Currently you cannot create a NIOPipeBootstrap when you are on
an event loop due to a precondition check. This check seeks to prevent
consumers of the API from passing in a file descriptor thats referencing
a file on disk or on the network.
fstat could potentially block the current event loop especially if the file descriptor is referencing a file over the network.
This check however prevents certain use cases of SwiftNIO, where the downsides
of a consumer of this api blocking their own event loop do not weigh in against preventing
an entire use case from SwiftNIO.
Modifications:
Removed the precondition
Result:
After this change it will be possible to feed file descriptors into the bootstrap which potentially block the current event loop.
A potential (portable) replacement for fstat still has to be found in order to solve this problem completely.