Race condition in muxnote lifecycle on Windows

[hmelder]

In event_windows.c, reference counting seems to be used to model a happens-before relationship:

Thread A is currently in _dispatch_event_loop_drain and waits on a completion event. Completion events are emitted by handlers such as the _dispatch_pipe_monitor_thread. Let thread B be such a thread. Before thread B posts the completion event, the reference count of the muxnote is incremented with _dispatch_muxnote_retain. This is an atomic increment with relaxed memory order.

Thread A receives the completion event and may call _dispatch_event_merge_pipe_handle_read which releases the muxnote. A comment above this release states that the muxnote is "[r]etained when posting the completion packet", but this happens in Thread B.

Therefore, when _dispatch_muxnote_release is called, the retain operation may not be visible to the thread performing the release, resulting in a disposal of the muxnote despite still being used.

If I understand this comment and the logic correctly, there needs to be some form of synchronisation between thread A and thread B and the reference counting does not provide this.

See:
#844 and #833

CC: @compnerd

[compnerd]

The relaxed is sufficient for atomic refcount handling, not the thread synchronisation. I think that the _dispatch_muxnote_release needs the acq_rel ordering to decrement the refcount. This would ensure that the acquire semantics in the _dispatch_pipe_monitor_thread are not violated.

[hmelder]

And on another note. There seems to be another race condition in the epoll implementation, where no reference counting of muxnotes happen. It might be a good idea to implement reference counting there. I would be willing to do that :D

[rokhinip]

cc: @akmorrison