Skip to content

Improvements and fixes#305

Merged
xinchen10 merged 41 commits into
masterfrom
master-work
Jul 9, 2026
Merged

Improvements and fixes#305
xinchen10 merged 41 commits into
masterfrom
master-work

Conversation

@xinchen10

Copy link
Copy Markdown
Member
  • Remove breaking changes
  • Performance improvements
  • Bug fixes

Each commit has the details.

xinchen10 and others added 30 commits July 2, 2026 11:13
Restore 23 symbols that were removed or made internal but are part of
the shipped public API (RS0017 fixes):
- Change 12 constants from internal to public
- Restore DefaultTimeout to static readonly
- Re-add 10 removed constants (AddRule, AmqpFormat, BadCommand, etc.)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Make FormatCode struct public to match shipped API surface. Add XML
doc comments for all public members (RS0017 fix).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Make SerializableType nested classes public (Primitive, Serializable,
Converted, Object, Collection, List, Map, Composite, CompositeList,
CompositeMap). Make AmqpType enum, SerialiableMember, MemberAccessor,
MethodAccessor, and IEncoding public to match shipped API surface.
Restore CreatePrimitiveType and CreateInstance to public.

Restore non-generic EncodingBase abstract class with shipped API:
constructor, FormatCode property, abstract methods (GetObjectEncodeSize,
EncodeObject, DecodeObject), and static VerifyFormatCode overloads.
EncodingBase<T> now inherits from EncodingBase. Primitive takes
EncodingBase parameter matching shipped signature.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove Default instance, Provider is null by default
- Restore all public virtual methods with original object-typed signatures (no-op)
- Internal static On* methods delegate to Provider if set, else log to AmqpEventSource
- Add XML doc comments to all public members

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Shipped API declares Name and Code as static readonly fields. Master changed
them to const which is a binary-breaking change. Revert across 43 files in
Framing, Sasl, and Transaction namespaces. Restore Properties.*Name fields.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove IEncoding interface; move abstract methods to EncodingBase
- Make AmqpEncoding methods public per shipped API surface
- Add GetEncoding(object) overload returning EncodingBase
- Rewrite ArrayEncoding to always use Array32 format
- Add DecodeArray/EncodeArray/GetArrayEncodeSize internal abstracts
  on EncodingBase to avoid boxing in untyped array paths
- Fix DescribedEncoding.GetArrayItemSize to use array-aware sizing
  instead of standalone-size-minus-FormatCode approximation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Restore shipped API surface for Terminus and the extension method.
Both marked [Obsolete] to guide users toward Source/Target directly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Offset/Length: internal int -> public long
- DecodeDescriptor: private static -> public static
- GetValueEncodeSize/EncodeValue/DecodeValue: internal virtual -> public virtual
- Update overrides in AmqpValue, Data, DescribedMap, DescribedList
- Cast Offset/Length to int in AmqpMessage.WriteBytes call

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add public GetString(ArraySegment<byte>) with branchless hex conversion
- Restore RemoteChannel, IncompleteUnsettled, Dynamic(Attach), Clone (4 overloads),
  UpsertPropertyIfNotDefault, TrackingId, Transactional(Delivery) as obsolete
- Make GetString(IDictionary) an extension method

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- AsyncIO constructor, Transport, WriteBufferQueueSize, WriteBuffer: internal -> public
- AsyncWriter, AsyncBufferReader, AsyncBufferWriter, FrameBufferReader: internal -> public
- Add WriteBuffer(IList<ByteBuffer>) on AsyncIO and AsyncWriter (obsolete)
- Restore AsyncFrameWriter class from hotfix (obsolete)
- Add XML doc comments for all public members

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add Amqp100, AmqpTls100, AmqpSasl100 static readonly fields.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Bulk visibility changes across 28 files to match shipped API surface:
- AmqpObject: ThisLock, NotifyOpening, CompleteOpen, CompleteClose, etc. internal->public
- AmqpLink: OnFlow, SendDelivery, SendFlow, etc. internal->public
- AmqpSession: SendCommand, OnAcceptTransfer, TryCreateRemoteLink internal->public
- AmqpSettings: 6 methods internal->public
- Address/MessageId: abstract members and static methods internal->public
- DescribedList/DescribedMap: DecodeValue internal->public
- EnsureRequired: internal->protected (base virtual + 14 overrides)
- Multiple, RestrictedMap, Frame, Delivery: various members internal->public
- SaslTransport, AmqpContractSerializer: methods internal->public

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Rename GetPayload/AddPayload(ByteBuffer) to GetPayloadBuffer/AddPayloadBuffer (internal)
- Add shipped abstract GetPayload(int, out bool) -> ArraySegment<byte>[] (obsolete)
- Add shipped virtual AddPayload(ByteBuffer, bool) with throw body
- Add shipped protected abstract OnCompletePayload(int)
- Change CompletePayload from virtual to non-virtual, calls OnCompletePayload
- BytesTransfered: internal set -> protected set
- Add RawByteBuffers property, Dispose(bool) disposes them
- Add PrepareForSend() (obsolete)
- Segments: public -> internal

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Visibility changes:
- Frame.HeaderSize: internal->public
- DescribedList.FieldCount: protected abstract + overrides in all subclasses
- AmqpLink: ProcessTransfer, CreateDelivery public; OnReceiveStateOpenSent added [Obsolete]
- ReceivingAmqpLink/SendingAmqpLink: CreateDelivery public override
- RequestResponseAmqpLink: ReceivingLink/SendingLink public
- SaslTransport: innerTransport public readonly, RequiresCompleteFrames override
- TlsTransport: sslStream public readonly
- TlsTransportSettings: innerSettings public readonly
- TransportBase: RequiresCompleteFrames virtual property added

Missing members restored:
- AmqpObject: DefaultOpenTimeout/DefaultCloseTimeout, MutualCapabilities, FindMutualCapabilites
- AmqpObjectState: Faulted = 13
- AmqpSession: CachedRemoteChannel, Links, LinksByRemoteHandle properties
- ByteBuffer: Clone(), InitBufferManagers() [Obsolete]
- AmqpSymbol/AmqpMap: ValueSize [Obsolete]
- DescribedType: Descriptor setter
- AmqpBitConverter: PeekUInt, WriteUByte [Obsolete]
- AmqpCodec: DecodeMap
- AmqpSessionSettings: Create(Begin)
- Data: GetEncodedPrefix [Obsolete]
- Error: FromException, IncludeErrorDetails
- TransportAsyncCallbackArgs: SetBuffer(ByteBuffer) [Obsolete]
- Singleton: TaskCompletionSource property
- AmqpContractSerializer: ReadObjectInternal/WriteObjectInternal [Obsolete]
- RestrictedMap: GetKey virtual
- ReceivingAmqpLink: DisposeMessageAsync, DrainAsyc, SetCacheSizeInBytes, TotalCacheSizeInBytes
- WebSocketTransport: constructor(WebSocket, Uri) [Obsolete]

Missing constructors: Source(Uri), Target(Uri) [Obsolete]
Missing types: CbsConstants (public), IAmqpProvider, INodeFactory [Obsolete]

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rename existing EndPoint properties to Local/Remote (internal) to avoid
conflict, then add shipped public abstract string LocalEndPoint/RemoteEndPoint
with overrides in TcpTransport, SaslTransport, TlsTransport, WebSocketTransport.
AmqpConnectionBase.LocalEndpoint/RemoteEndpoint now returns string.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix double BOM in PublicAPI.Shipped.txt
- Remove inherited members from Shipped.txt (AmqpMap, Multiple<T>)
- AmqpLink.UnsettledMap: IDictionary -> Dictionary return type
- ReceivingAmqpLink: Add AcceptMessage(AmqpMessage, bool) overload
- AmqpSession: Add SendCommand/TrySendTransfer ArraySegment overloads [Obsolete]
- Move RegisterCreditListener from AmqpLink to SendingAmqpLink
- Add internal virtual NotifyCreditAvailable pattern
- Use CompareExchange for all Register*Listener methods

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… base

Replace Dictionary<Type,TItem> base with KeyedCollection<Type,TItem> to match
shipped API surface. Mark KeyedByTypeCollection and Extensions property as
[Obsolete]. Add CS0436 pragmas to suppress potential conflict with
System.ServiceModel.Primitives WCF package. Remove AddExtension/TryGetExtension
extension methods; caller uses connection.Extensions directly.
Fix XML doc paramref -> typeparamref for type parameters.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reimplement 5 factory methods that were throwing NotSupportedException:
- CreateAmqpStreamMessage(Stream, Stream, bool)
- CreateAmqpStreamMessageBody(Stream)
- CreateAmqpStreamMessageHeader(BufferListStream)
- CreateInputMessage(BufferListStream)
- CreateOutputMessage(BufferListStream, bool)

All methods now convert stream data to ByteBuffer and create
AmqpBufferMessage, matching the pattern already used by
CreateAmqpStreamMessage(BufferListStream, bool).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Internalize ~113 RS0016 symbols that were not in Shipped.txt:
- 6 entire types made internal (AmqpInMemoryTerminusStore,
  AmqpLinkIdentifier, AmqpLinkTerminus, IAmqpTerminusStore,
  LinkTerminusExpiryPolicy, TxnCapabilities)
- ~107 individual members on shipped public types
- Protected methods changed to internal where applicable
- Unshipped.txt declares 12 symbols that must remain public
  (interface members, operators, enum values, IEquatable)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add XML documentation to all public members in CbsConstants,
  MemberAccessor, MethodAccessor, SerialiableMember, StateTransition,
  and SerializableType to resolve CS1591 warnings
- Fix test compilation after RS0016 internalization:
  - TestAmqpBroker: IAmqpTerminusStore access, BrokerMessage abstract overrides
  - CancellationTokenTests: CreateDelivery/ProcessUnsettledDeliveries access
  - AmqpUtils/AmqpMessageTests: GetPayload -> GetPayloadBuffer

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…eAsync public

- AmqpConnectionFactory: remove internal ctor, make Settings property public
- AmqpObject: make OpenAsync() and CloseAsync() (default timeout) public
- SendingAmqpLink: make SendMessageAsync public
- ReceivingAmqpLink: make ReceiveMessageAsync public
- Update callers to use factory.Settings pattern
- Update PublicAPI.Unshipped.txt

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GetPayloadBuffer was returning this.Buffer directly (shared reference).
When Unlock() called Buffer.Seek(), it mutated the shared ByteBuffer
while async writes were still in flight, causing frame misalignment
and 'Invalid frame size' errors during connection close.

Use Delivery.GetPayload() which returns a GetSlice() - an independent
ByteBuffer with its own offset/length, safe from concurrent mutation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace xunit/xunit.runner.visualstudio with MSTest 3.8.3
- Convert [Fact] to [TestMethod], add [TestClass] to all test classes
- Convert Assert.Equal/True/False/Null/NotNull to MSTest equivalents
- Convert TestAmqpBrokerFixture to [AssemblyInitialize]/[AssemblyCleanup]
- Remove IClassFixture, convert constructors to [TestInitialize]
- Convert IDisposable.Dispose to [TestCleanup]
- Add AssertExtensions.cs for ThrowsAny/ThrowsAnyAsync (no MSTest equiv)
- Use global:: prefix on MSTest using to avoid namespace collision
- Replace [assembly: CollectionBehavior] with [assembly: DoNotParallelize]

All 170 tests pass on net462.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Port of hotfix commits 0bdd59a, 87a3971, 8796c86:
- Default to SslProtocols.None (OS picks best protocol, enables TLS 1.3)
- Add BeginAuthenticateWithRetry to fall back to legacy TLS 1.0/1.1/1.2
  on older .NET platforms that don't support SslProtocols.None
- Respect CheckCertificateRevocation setting instead of hardcoding
- Preserve user-set vs system-default distinction via UserProtocols

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Port of hotfix commit f8907eb. Introduces EncodingCache with:
- BoxedCache<T>: hash-based cache for AmqpSymbol boxing (capacity 40)
- UlongCache: pre-boxed ranges for AMQP descriptor codes

AmqpDescribed constructor now uses cached boxed ulong instead of
boxing on every creation. RestrictedMap subclasses (Fields, FilterSet,
Annotations) override GetKey to use cached boxed AmqpSymbol keys.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…geId

Port of hotfix commit 2049688. Added DecodeObject overrides in
BooleanEncoding, IntEncoding, SymbolEncoding, and ULongEncoding to
return cached boxed values instead of boxing on every decode call.
Rewrote MessageId.Decode() to switch on format code directly,
avoiding boxing entirely. Added bool/int caches and null-safe
symbol boxing to EncodingCache.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Port of hotfix commit e557bf0. RestrictedMap now inherits from
AmqpMap directly instead of wrapping an inner map, eliminating one
dictionary allocation per ApplicationProperties/Annotations instance.
DecodeMap<T> decodes directly into the target map.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
xinchen10 and others added 11 commits July 2, 2026 11:13
Port of hotfix commit 843ab89. When the inner transport is closed during
AMQP handshake, callers now get a meaningful IOException instead of a raw
ObjectDisposedException.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Port of hotfix commit a4b111d. Replaces single-tier BoxedCache<T> with a
3-tier KeyValueCache<TKey,TValue> using lock-free population. Adds an
encoded symbol cache that avoids repeated ASCII string allocation during
AMQP decoding. Simplifies int cache to a pre-boxed array for values 0-7.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Port hotfix 05317b6: Replace TryRemoveWork with RemoveWork that verifies
work identity. If a different work occupies the same key, the victim is
canceled to prevent stuck operations. DisposeAsyncResult now only calls
StartTracking when DisposeDelivery succeeds, and Cancel/CompleteOnTimer
always complete regardless of remove outcome. Added descriptive messages
to OperationCanceledException throughout.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Port hotfix 354669c: Add state machine to prevent CancellationTokenRegistration
and Timer from leaking when CompleteInternal races with StartTracking. Resources
are only disposed when both sides agree they exist (state transitions through
created→starting→active→completed).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Port of hotfix commit 695f85a. When the ConnectAsync task completes
in faulted or canceled state, call cws.Abort() before setting the
exception. This ensures the underlying WebSocket is properly cleaned
up and prevents unobserved task exceptions from lingering connections.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Port of hotfix afe233a. Consolidates error handling in WebSocketTransport
completion paths:
- OnWriteComplete/OnReadComplete now accept Task parameter and handle
  faulted/canceled states internally instead of assuming success
- Fix sync paths where task.IsCompleted was treated as success without
  checking IsFaulted/IsCanceled
- CloseInternal sync path now properly throws on fault/cancel
- WebSocketTransportInitiator uses Dispose instead of Abort for more
  thorough cleanup on fault/cancel

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Port of hotfix commit 20d46f2. The encodingsByCode dictionary was
missing entries for FormatCode.Decimal32 and FormatCode.Decimal64,
which would cause NotSupportedException when decoding arrays
containing these format codes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The lock-free optimization in SyncEvent/TryComplete had a race where:
1. End() creates a ManualResetEventSlim but hasn't stored it yet
2. TryComplete sets isCompleted=1, reads manualResetEvent as null, skips Set()
3. The event is stored unsignaled, End() waits forever

Fix: after CompareExchange stores the event, re-check IsCompleted and signal
if TryComplete already ran. This is safe because CompareExchange is a full
memory barrier, so the subsequent IsCompleted read sees TryComplete's write.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Prevents newer SDKs (e.g., 10.0.x) from being picked up locally, which
triggers workload auto-imports for iOS/MAUI even though this repo only
targets net462 and net8.0. Matches the SDK version CI uses via
setup-dotnet dotnet-version: 8.0.x.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
TlsTransport previously merged multi-buffer writes into a single ByteBuffer
then called SslStream.BeginWrite, which internally copies the plaintext
again into its own encryption buffer.

Move the accumulation into TransportStream: SslStream synchronously
encrypts each source segment in place and hands the ciphertext to
TransportStream.Write, which appends into an auto-growing ByteBuffer
without any I/O. TlsTransport then issues a single async I/O via
TransportStream.BeginFlushWrite to send the accumulated ciphertext to
the inner transport. Buffer ownership stays with TransportStream and is
released in EndWrite so an in-flight socket write never reads freed bytes.

Also drop the >0 count assert in TcpTransport.ReadAsync and short-circuit
count==0 in TransportStream.ReadAsync. .NET 8 SslStream issues zero-byte
reads to await readability, which the old assert misinterpreted as a bug.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Port hotfix commit e894c2c to master's post-refactor encoding architecture.

- Track cumulative size of unbounded (zero-width) elements per decode
  call, capped at MaxUnboundedSize (64KB), to prevent excessive
  allocations from types like uint0/ulong0/list0/booleanTrue/False
  without penalizing legitimate large collections.
- Enforce MaxNestingDepth (64) across described, list, map, and array
  decoding to prevent stack overflow from deeply nested compound types.
- Cross-validate declared size/count against remaining buffer length in
  ReadSizeAndCount and ReadCount. Rejects negative counts (uint values
  larger than int.MaxValue).
- Thread ref totalUnboundedSize and depth through the decode chain via
  new internal virtual DecodeObject/DecodeArray overloads on
  EncodingBase and EncodingBase<T>. Compound types (List, Map,
  Described, Array) override to thread state through child decodes.
  Public API signatures unchanged, preserving binary compatibility.
- For nested compound array elements (Array-of-Array, Array-of-List,
  Array-of-Map), bypass the base ReadArrayValue path in DecodeArray so
  depth and unbounded-size tracking are not reset per element. Described
  arrays are handled by an override on DescribedEncoding that preserves
  the descriptor-once + N-values wire format.
- Overflow-safe multiply when crediting count * unboundedElementSize.
- Fix pre-existing TryGetEncoding bug: typeof(System.Array) fell through
  to the IList branch because Array.IsAssignableFrom(typeof(IList)) is
  true, returning ListEncoding instead of ArrayEncoding. Added explicit
  typeof(System.Array) check ordered before the IList branch.

Ports 30 xUnit tests from hotfix commit e894c2c to MSTest and adjusts
for master's API surface (removes a call to
AmqpCodec.DecodeArray<uint[]> which is a pre-existing limitation on
master's generic dispatch and unrelated to the safeguards).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sjkwak sjkwak self-assigned this Jul 9, 2026
@xinchen10
xinchen10 merged commit 24b018a into master Jul 9, 2026
2 checks passed
@xinchen10
xinchen10 deleted the master-work branch July 9, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants