Contain consumer exceptions in D-Bus signal handlers, and stop disposing from finalizers - #83
Open
Kinchul wants to merge 7 commits into
Open
Conversation
BlueZ answers org.bluez.Error.DoesNotExist when the advertisement or the application it is asked to unregister is already gone, which a bluetoothd or DBus restart makes routine. Unregistering has then reached its goal, so the error is swallowed instead of failing the teardown and, through Dispose(), the caller that only wanted the server gone.
Dispose() reported its own success on stderr, so every teardown surfaced as an error line in the consumer's log. Debug.WriteLine matches the other traces of the class and compiles out of release builds.
~Agent() ran Dispose(), which issues an UnregisterObject call on a connection the agent does not own and may already have been disposed by its owner. ~AgentManager() called a Dispose() whose only statement was SuppressFinalize. Both objects are IDisposable, so their cleanup belongs to their owner.
Adapter and Device finalizers called the public Dispose, so the finalizer thread disposed managed watchers and issued DBus traffic on the shared system connection. Removed both finalizers and the now-dead GC.SuppressFinalize calls. Callers that never dispose these objects now leak a signal-handler delegate instead of having it released non-deterministically. Added BlueZManager.GetAdapterProxiesAsync so a caller can select an adapter without GetAdaptersAsync registering three watchers for every adapter on the bus, including the ones it discards. Adapter.CreateAsync is now public so an external assembly can build the adapter it keeps. GetAdaptersAsync is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Tmds.DBus reads signals on one receive loop. When a handler throws,
ReceiveMessagescatchesit, calls
Disconnect()and clears the connection's signal handlers. WithAutoConnecttheconnection then transparently reconnects for method calls only: property reads keep
working, every event is silent, and nothing is logged. On our gateway this presented as BLE
scans that found nothing for hours, with a healthy-looking process.
A consumer callback throwing is not the library's fault, but taking the whole connection down
silently is a bad failure mode for it to have.
What changed
subscriber can no longer kill signal delivery for the process.
Adapter,Device,GattServer,AgentandAgentManagerfinalizers. Theycalled the public
Dispose, so the finalizer thread disposed managed watchers and issuedD-Bus traffic — on a connection it may not own and which may already be disposed. Cleanup
belongs to the owner; these types are all
IDisposable.GattServer.Disposeno longer fails when BlueZ answersorg.bluez.Error.DoesNotExist:a bluetoothd restart makes that routine, and unregistering an object that is already gone
has reached its goal.
GattServer.Disposetraces its success withDebug.WriteLineinstead of reporting it onstderr.
BlueZManager.GetAdapterProxiesAsyncand madeAdapter.CreateAsyncpublic, so acaller can pick an adapter without
GetAdaptersAsyncregistering three watchers for everyadapter on the bus, including the ones it discards.
GetAdaptersAsyncis unchanged.Behaviour is otherwise unchanged; no public API is removed.