Dispose viewConnectable synchronously

[kmcbride]

Asynchronously disposing the controller view connection can lead to unexpected behavior (and ultimately crashes) in certain scenarios. It also differs from how other types (effect handler, event source, loop) are disposed: synchronously on the loop queue.

This contrived example demonstrates how the current implementation will lead to a crash:

// scenario: executing on main queue, ConnectableClass previously attached via .connectView()
loopController.start()

// queues work for next loop pass
DispatchQueue.main.async {
    // crashes in ConnectableClass .connect() because it executes before the queued dispose
    loopController.start()
}

// stops loop but queues ConnectableClass connection dispose on the acceptQueue (main)
loopController.stop()

As I wasn't involved in the original design it's possible I'm missing some important context, but I've attempted to annotate my reasoning for the changes as review comments.

[codecov]

Codecov Report

Merging #190 (3ef5151) into master (fd1fb15) will decrease coverage by 0.39%.
The diff coverage is 59.09%.

@@            Coverage Diff             @@
##           master     #190      +/-   ##
==========================================
- Coverage   95.37%   94.97%   -0.40%     
==========================================
  Files          42       42              
  Lines        1427     1414      -13     
==========================================
- Hits         1361     1343      -18     
- Misses         66       71       +5     

Flag	Coverage Δ
macspm	94.97% <59.09%> (-0.40%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
MobiusCore/Source/Lock.swift	100.00% <ø> (ø)
MobiusCore/Source/MobiusController.swift	96.05% <50.00%> (-1.98%)	⬇️
...iusCore/Source/AsyncDispatchQueueConnectable.swift	84.61% <66.66%> (-9.14%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd1fb15...3ef5151. Read the comment docs.

[kmcbride]

The previous code had an issue with retaining the consumer reference, which would lead to a somewhat nebulous crash when its unowned loop reference was accessed by emitting an event after disposal. This code is functionally equivalent, but the cause of the error should be clearer now.

[kmcbride]

Due to this closure being created and owned by the loop—and the lifetime of the loop being tied to the controller's running state—this doesn't appear to be a valid scenario, unless the loop reference was somehow leaking.

[rastersize]

This makes sense to me. Might still be good to get some of the original Mobius.swift folks to look it over as you mentioned.

[kmcbride]

@JensAyton would you mind taking a look? I think you probably have the most insight into the existing implementation.