Fix empty array serialization and silent error swallowing#10
Conversation
…ndling Empty arrays of struct types (e.g. List<AtSpiObjectReference> with 0 items) caused signature inference failures during native marshaling. The server would throw during reply serialization, the error was silently swallowed, and the client would hang forever waiting for a reply that was never sent. Root cause: AppendArray re-inferred the D-Bus signature from the value even when the message already had a known signature set via SetBodyWithSignature. For empty arrays of IDBusStructConvertible, there is no element to peek at, so inference throws. Fixes: - Add CreateReplyWithSignature/CreateSignalWithSignature to DBusMessage - Add explicit-signature constructor to DBusVariant - Source generator emits compile-time-known signatures for method replies, property getters, and GetAllProperties - AppendBody uses the message's known signature to drive serialization, bypassing inference for arrays/dicts - HandleMessage in LibDBusWireWorker now fails pending TCS on deserialization errors instead of silently dropping them - NormalizePath failure now sends an error reply instead of silently returning - All diagnostics-only error paths now fall back to stderr when diagnostics is null, ensuring errors are never completely invisible
There was a problem hiding this comment.
Pull request overview
This PR fixes critical issues with empty array serialization and silent error handling in the D-Bus marshaling layer. The root cause was that empty arrays of struct types couldn't be inferred during serialization, causing the server to crash during reply serialization with the error being silently swallowed, leaving clients hanging indefinitely.
Changes:
- Added explicit-signature versions of reply and signal creation methods to
DBusMessageto bypass inference - Extended
DBusVariantwith an explicit-signature constructor for property values - Refactored serialization in
LibDBusMessageMarshalerto use known signatures when available - Enhanced error handling to fail pending TCS on deserialization errors and ensure all errors have visible logging
- Updated the source generator to emit compile-time-known D-Bus signatures for method replies, property getters, and
GetAllProperties
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/Avalonia.DBus/DBusMessage.cs |
Added CreateReplyWithSignature and CreateSignalWithSignature methods to enable explicit signature specification |
src/Avalonia.DBus/DBusVariant.cs |
Added explicit-signature constructor to support compile-time-known signatures for property values |
src/Avalonia.DBus/LibDBusMessageMarshaler.cs |
Refactored AppendBody to use known signatures; added AppendValueWithSignature, AppendArrayWithSignature, and AppendDictWithSignature methods |
src/Avalonia.DBus/LibDBusWireWorker.cs |
Enhanced error handling to fail pending TCS on deserialization errors; added fallback stderr logging via new LogOrTrace method |
src/Avalonia.DBus/DBusConnection.Worker.cs |
Added error replies for NormalizePath failures; enhanced error logging with stderr fallback |
src/Avalonia.DBus.SourceGen/DBusSourceGenerator.Handler.cs |
Updated handler generation to emit compile-time-known signatures for method replies and property values |
Now let me verify that there are no issues I might have missed by doing a final check on the logic. Looking at the code one more time to ensure correctness:
- ✓ The message's known signature is correctly used to drive serialization
- ✓ Error handling properly fails pending TCS on deserialization errors
- ✓ Stderr logging is used as fallback when diagnostics is null
- ✓ The source generator correctly computes and formats signatures
- ✓ The new API methods follow existing conventions
Everything looks good!
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Empty arrays of struct types (e.g.
List<AtSpiObjectReference>with 0 items) caused signature inference failures during native marshaling. The server would throw during reply serialization, the error was silently swallowed, and the client would hang forever waiting for a reply that was never sent.Root cause:
AppendArrayre-inferred the D-Bus signature from the value even when the message already had a known signature set viaSetBodyWithSignature. For empty arrays ofIDBusStructConvertible, there is no element to peek at, so inference throws.Fixes:
CreateReplyWithSignature/CreateSignalWithSignaturetoDBusMessageDBusVariantGetAllPropertiesAppendBodyuses the message's known signature to drive serialization, bypassing inference for arrays/dictsHandleMessageinLibDBusWireWorkernow fails pending TCS on deserialization errors instead of silently dropping themNormalizePathfailure now sends an error reply instead of silently returningstderrwhen diagnostics is null, ensuring errors are never completely invisible