event: ensure acquire release semantics for muxnote disposal

[compnerd]

While it is sufficient to use the relaxed ordering for the acquire semantics in the _dispatch_muxnote_retain, we need to use the acquire release semantics on the release in _dispatch_muxnote_release to ensure that any pending retains are not interrupted. This should hopefully alleviate the occasional crashes that have been observed with the muxnote reference counting.

Fixes: #887, #844

[compnerd]

@swift-ci please test

[hmelder]

There is an example in the boost docs where only release and an additional memory fence is used. Here is the rational behind it:

It would be possible to use memory_order_acq_rel for the fetch_sub operation, but this results in unneeded "acquire" operations when the reference counter does not yet reach zero and may impose a performance penalty.

Would it make sense to adopt a similar approach and use os_atomic_thread_fence?

#include <boost/intrusive_ptr.hpp>
#include <boost/atomic.hpp>

class X {
public:
  typedef boost::intrusive_ptr<X> pointer;
  X() : refcount_(0) {}

private:
  mutable boost::atomic<int> refcount_;
  friend void intrusive_ptr_add_ref(const X * x)
  {
    x->refcount_.fetch_add(1, boost::memory_order_relaxed);
  }
  friend void intrusive_ptr_release(const X * x)
  {
    if (x->refcount_.fetch_sub(1, boost::memory_order_release) == 1) {
      boost::atomic_thread_fence(boost::memory_order_acquire);
      delete x;
    }
  }
};

https://www.boost.org/doc/libs/1_57_0/doc/html/atomic/usage_examples.html#boost_atomic.usage_examples.example_reference_counters

[compnerd]

Yeah, that would be a bit cheaper on arm64 at least, and without too much more complexity; let me see if I can integrate that approach.

[compnerd]

@swift-ci please test

[compnerd]

@swift-ci please test

[compnerd]

Going to merge this for now - this bug has haunted us for a while, and it would be good to get this tested more broadly.