Handle null events by PostQueuedCompletionStatus

[ghost]

I am calling this:

PostQueuedCompletionStatus(wepoll_handle, 0, 0, NULL);

in order to wakeup threads blocked on epoll_wait. This patch makes wepoll report a null event (event=0, u64=0) instead of crashing.

[yorickpeterse]

@piscisaureus What are your thoughts on this change? For Rust we have a crate that wraps wepoll, but @stjepang needs these changes for some crate they are working on. This resulted in them having to fork the wepoll Rust crate. It would be nice (and less confusing to Rust users) if we could reduce this to one crate.

[ghost]

For this to really work, port_wait() also needs to be modified:

  /* Dequeue completion packets until either at least one interesting event
   * has been discovered, or the timeout is reached. */
  for (;;) {
    uint64_t now;

    result = port__poll(
        port_state, events, iocp_events, (DWORD) maxevents, gqcs_timeout);
    if (result < 0 || result > 0)
      break; /* Result, error, or time-out. */

    if (timeout < 0)
      continue; /* When timeout is negative, never time out. */

    /* Update time. */
    now = GetTickCount64();

    /* Do not allow the due time to be in the past. */
    if (now >= due) {
      SetLastError(WAIT_TIMEOUT);
      break;
    }

    /* Recompute time-out argument for GetQueuedCompletionStatus. */
    gqcs_timeout = (DWORD)(due - now);
  }

If port__poll() reports zero events, we should break out of the loop if a null event caused it to return 0.

[bnoordhuis]

Just an interested bystander, no real stake, but real epoll never generates null events. The way you solve it on linux is by either adding an eventfd or waking it up from the system call by means of a signal (which causes it to return with an EINTR error.)

[ghost]

So, I don't really care what implementation method is used here but there needs to be some way of unblocking calls to epoll_wait(). :)

That could be using eventfd or PostQueuedCompletionStatus or whatever, but right now the only way that I know of is by creating a TCP self-pipe and then triggering a one-byte write to trigger an I/O event. Unfortunately, that occupies a network port that is available to other processes, so it isn't a particularly clean solution.

In fact, the first version of https://github.com/stjepang/smol used a TCP self-pipe but enough people complained that I just patched wepoll myself and switched to this PostQueuedCompletionStatus hack.

[piscisaureus]

So what about #8 (comment) ?

[yorickpeterse]

@piscisaureus

Regarding naming of custom functions, I think sticking with the epoll_ prefix keeps things consistent. It's a bit odd to have a bunch of epoll_X functions, and then suddenly have one or two wepoll_X functions. For your mentioned posting example, I think epoll_post makes the most sense.

Per consideration (5), should we mandate that the posted event has the EPOLLONESHOT flag set, or otherwise it fails with EINVAL? Alternatively, EPOLLONESHOT might be implied.

I think requiring the user to set this explicitly may be the most future proof. If this option is implied by default, making it optional in the future would require changing all existing uses of the function. Requiring it to be set by the user would not require such changes, and make the upgrading process easier.