Skip to content

Refactor Win32 iocp evloop#17101

Open
ysbaddaden wants to merge 2 commits into
crystal-lang:masterfrom
ysbaddaden:refactor/win32-iocp-evloop
Open

Refactor Win32 iocp evloop#17101
ysbaddaden wants to merge 2 commits into
crystal-lang:masterfrom
ysbaddaden:refactor/win32-iocp-evloop

Conversation

@ysbaddaden

@ysbaddaden ysbaddaden commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Fixes BUG 1 and BUG 2 from #16675.

Creates a global IOCP instance, and associates every FILE and SOCKET handles to that global IOCP instance. Every OVERLAPPED operation is associated to the local IOCP instance.

Starts a thread that receives the completion events and forwards them to the local IOCP instance of the event loop that created the OVERLAPPED operation.

NOTE: this is required for the IOCP event loop to work with Execution Contexts.

Closes #16675 (BUG 3 can't be fixed.)

@ysbaddaden ysbaddaden self-assigned this Jun 26, 2026
@ysbaddaden ysbaddaden added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:runtime labels Jun 26, 2026
@ysbaddaden ysbaddaden force-pushed the refactor/win32-iocp-evloop branch from f32e0eb to 81fa6d3 Compare June 29, 2026 16:36
@ysbaddaden ysbaddaden marked this pull request as ready for review June 29, 2026 16:36
@ysbaddaden ysbaddaden added this to the 1.21.0 milestone Jun 29, 2026
@ysbaddaden

ysbaddaden commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator Author

There could be a better solution, maybe using queues and internal notifications to not go through IOCP again, for example a single global IOCP, lock-free singly-linked queues and SetEvent to interrupt + WaitableTimer for timers + WaitForMultipleObjects to wait, but that would require a much larger refactor.

@ysbaddaden ysbaddaden force-pushed the refactor/win32-iocp-evloop branch from 81fa6d3 to d07b73d Compare June 30, 2026 11:10
@ysbaddaden

ysbaddaden commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased from master and updated the :execution_context flag checks.

We must assign every File and Socket to an IOCP instance for its whole
lifetime, but we can start multiple event loops, each with its own IOCP
instance, meaning that opening a file or socket in evloop A then reading
and writing in evloop B would keep sending the events to evloop A.

With execution contexts this isn't acceptable:

1. a context can shutdown (e.g. the isolated context) and evloop A
   disappear: the would no more valid evloop to receive IOCP
   notifications for the file or socket;

2. every evloop is expected to recieve events for its context only (not
   others) so we can enqueue locally;

3. if the threads running evloop A are busy they can block the fibers of
   evloop B.
@ysbaddaden ysbaddaden force-pushed the refactor/win32-iocp-evloop branch from d07b73d to eb780d3 Compare July 2, 2026 10:19
@ysbaddaden

ysbaddaden commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

CI suddenly broke after a rebase from master to bring in execution contexts from #17100 (not available) and the merged #17098 and #17099 that already were.

A compiler on windows built with the change seemed to start failing (or at least hang), but execution contexts were already manually enabled, and both #17098 and #17099 were already in the branch... A difference is that execution contexts are now enabled by default, and we miss #17104.

I rebased again to bring it in. Let's see.

I forgot to fix the flag?(:execution_context) and the forwarder thread was never started 🤦

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:multithreading topic:stdlib:runtime

Projects

Status: Review

Development

Successfully merging this pull request may close these issues.

Windows/IOCP: handles are tied to a specific IOCP instance

1 participant