Better handle writability changes in triggerWriteOperations

[glbrntt]

Motivation:

If a write is buffered in the PendingWritesManager and the high
watermark is breached then a writability change will be fired as a
result. The corresponding event when the channel becomes writable again
comes when enough pending bytes have been written to the socket (i.e. as
a result of a flush()) and we fall below the low watermark.

However, if a promise associated with a write has a callback
registered on its future which writes enough data to breach the high
watermark (and also flushes) then we will re-entrantly enter
flushNow(). This is okay, flushNow() protects against re-entrancy
and any re-entrantly enqueud flushed writes will be picked up in the
write-spin-loop.

The issue here is that the writeSpinLoop does not monitor for changes
in writability. Instead if checks the writability before it begins and
after it completes, only signalling if there was a change. This is
problematic: the re-entrant write-and-flush could have caused the
channel to become unwritable yet no event will be fired to make it
writable again!

Modifications:

• Check whether the channel became unwritable after each write
  operation, and emit a writability change if the channel became
  unwritable and writable again during the spin loop.

Result:

• Better protection against re-entrant writability changes.

[Lukasa]

So the downside of this is that we are now repeatedly polling the atomic isWritable variable. This is a technically correct solution, so we may need to merge it if we can't find a better one, but I'd like to look at alternative options.

One thing I'm wondering about is whether we can defer writability changes that occur while we're already in triggerWriteOperations. Essentially, if we're in triggerWriteOperations, perhaps add should never issue a writability notification.

Another option that preserves these writability notifications (which, to be clear, I believe are useful) is to have a state variable updated by add if it does emit a false writability state update. That state variable can be checked by this code at the bottom of the loop. This is probably the easiest to implement and simplest to understand: maybe we should do that?

What do you think @glbrntt?

[glbrntt]

So the downside of this is that we are now repeatedly polling the atomic isWritable variable.

Agreed, it's far from ideal!

One thing I'm wondering about is whether we can defer writability changes that occur while we're already in triggerWriteOperations. Essentially, if we're in triggerWriteOperations, perhaps add should never issue a writability notification.

Another option that preserves these writability notifications (which, to be clear, I believe are useful) is to have a state variable updated by add if it does emit a false writability state update. That state variable can be checked by this code at the bottom of the loop. This is probably the easiest to implement and simplest to understand: maybe we should do that?

I think it's better to preserve the writability notifications, I'll give this second approach a shot.

[glbrntt]

I had to add this to work around triggerWriteOperations being an extension (since it mutates self), making it a mutating func didn't do the trick either.

[Lukasa]

Great fix @glbrntt, I really like it.