Skip to content

Commit 0866269

Browse files
committed
Remove TODO
Add assertion Remove assertion, add comment * Add `ConfirmsAreEnabled` in `ChannelBase` * Add `CancellationToken` to `CreateChannelAsync` * Use `CancellationToken` argument in `OpenAsync`
1 parent f9dd5c6 commit 0866269

10 files changed

+66
-47
lines changed

projects/RabbitMQ.Client/PublicAPI.Unshipped.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -919,7 +919,7 @@ virtual RabbitMQ.Client.TcpClientAdapter.ReceiveTimeout.set -> void
919919
~RabbitMQ.Client.IChannel.TxRollbackAsync() -> System.Threading.Tasks.Task
920920
~RabbitMQ.Client.IChannel.TxSelectAsync() -> System.Threading.Tasks.Task
921921
~RabbitMQ.Client.IConnection.CloseAsync(ushort reasonCode, string reasonText, System.TimeSpan timeout, bool abort, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task
922-
~RabbitMQ.Client.IConnection.CreateChannelAsync() -> System.Threading.Tasks.Task<RabbitMQ.Client.IChannel>
922+
~RabbitMQ.Client.IConnection.CreateChannelAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<RabbitMQ.Client.IChannel>
923923
~RabbitMQ.Client.IConnection.UpdateSecretAsync(string newSecret, string reason) -> System.Threading.Tasks.Task
924924
~RabbitMQ.Client.IConnectionFactory.CreateConnectionAsync(string clientProvidedName, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<RabbitMQ.Client.IConnection>
925925
~RabbitMQ.Client.IConnectionFactory.CreateConnectionAsync(System.Collections.Generic.IEnumerable<RabbitMQ.Client.AmqpTcpEndpoint> endpoints, string clientProvidedName, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<RabbitMQ.Client.IConnection>

projects/RabbitMQ.Client/client/api/IConnection.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,13 +225,14 @@ public interface IConnection : INetworkConnection, IDisposable
225225
/// <param name="reasonText">A message indicating the reason for closing the connection.</param>
226226
/// <param name="timeout"></param>
227227
/// <param name="abort">Whether or not this close is an abort (ignores certain exceptions).</param>
228-
/// <param name="cancellationToken"></param>
229-
Task CloseAsync(ushort reasonCode, string reasonText, TimeSpan timeout, bool abort, CancellationToken cancellationToken = default);
228+
/// <param name="cancellationToken">Cancellation token</param>
229+
Task CloseAsync(ushort reasonCode, string reasonText, TimeSpan timeout, bool abort,
230+
CancellationToken cancellationToken = default);
230231

231232
/// <summary>
232233
/// Asynchronously create and return a fresh channel, session, and channel.
233234
/// </summary>
234-
// TODO cancellation token
235-
Task<IChannel> CreateChannelAsync();
235+
/// <param name="cancellationToken">Cancellation token</param>
236+
Task<IChannel> CreateChannelAsync(CancellationToken cancellationToken = default);
236237
}
237238
}

projects/RabbitMQ.Client/client/impl/AutorecoveringChannel.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ public IBasicConsumer DefaultConsumer
146146
public string CurrentQueue => InnerChannel.CurrentQueue;
147147

148148
internal async Task AutomaticallyRecoverAsync(AutorecoveringConnection conn, bool recoverConsumers,
149-
bool recordedEntitiesSemaphoreHeld = false)
149+
bool recordedEntitiesSemaphoreHeld, CancellationToken cancellationToken)
150150
{
151151
if (false == recordedEntitiesSemaphoreHeld)
152152
{
@@ -156,7 +156,7 @@ internal async Task AutomaticallyRecoverAsync(AutorecoveringConnection conn, boo
156156
ThrowIfDisposed();
157157
_connection = conn;
158158

159-
RecoveryAwareChannel newChannel = await conn.CreateNonRecoveringChannelAsync()
159+
RecoveryAwareChannel newChannel = await conn.CreateNonRecoveringChannelAsync(cancellationToken)
160160
.ConfigureAwait(false);
161161
newChannel.TakeOver(_innerChannel);
162162

projects/RabbitMQ.Client/client/impl/AutorecoveringConnection.Recording.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -731,15 +731,15 @@ private void RecordChannel(in AutorecoveringChannel channel,
731731
}
732732

733733
private async Task RecordChannelAsync(AutorecoveringChannel channel,
734-
bool channelsSemaphoreHeld = false)
734+
bool channelsSemaphoreHeld, CancellationToken cancellationToken)
735735
{
736736
if (channelsSemaphoreHeld)
737737
{
738738
DoAddRecordedChannel(channel);
739739
}
740740
else
741741
{
742-
await _channelsSemaphore.WaitAsync()
742+
await _channelsSemaphore.WaitAsync(cancellationToken)
743743
.ConfigureAwait(false);
744744
try
745745
{
@@ -757,6 +757,7 @@ private void DoAddRecordedChannel(AutorecoveringChannel channel)
757757
_channels.Add(channel);
758758
}
759759

760+
// TODO remove this unused method
760761
internal void DeleteRecordedChannel(in AutorecoveringChannel channel,
761762
bool channelsSemaphoreHeld, bool recordedEntitiesSemaphoreHeld)
762763
{

projects/RabbitMQ.Client/client/impl/AutorecoveringConnection.Recovery.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ await RecoverBindingsAsync(_innerConnection, recordedEntitiesSemaphoreHeld: true
184184
.ConfigureAwait(false);
185185

186186
}
187-
await RecoverChannelsAndItsConsumersAsync(recordedEntitiesSemaphoreHeld: true)
187+
await RecoverChannelsAndItsConsumersAsync(recordedEntitiesSemaphoreHeld: true, cancellationToken: cancellationToken)
188188
.ConfigureAwait(false);
189189
}
190190
finally
@@ -541,7 +541,7 @@ void UpdateConsumer(string oldTag, string newTag, in RecordedConsumer consumer)
541541
}
542542
}
543543

544-
private async ValueTask RecoverChannelsAndItsConsumersAsync(bool recordedEntitiesSemaphoreHeld = false)
544+
private async ValueTask RecoverChannelsAndItsConsumersAsync(bool recordedEntitiesSemaphoreHeld, CancellationToken cancellationToken)
545545
{
546546
if (false == recordedEntitiesSemaphoreHeld)
547547
{
@@ -551,7 +551,8 @@ private async ValueTask RecoverChannelsAndItsConsumersAsync(bool recordedEntitie
551551
foreach (AutorecoveringChannel channel in _channels)
552552
{
553553
await channel.AutomaticallyRecoverAsync(this, _config.TopologyRecoveryEnabled,
554-
recordedEntitiesSemaphoreHeld: recordedEntitiesSemaphoreHeld)
554+
recordedEntitiesSemaphoreHeld: recordedEntitiesSemaphoreHeld,
555+
cancellationToken: cancellationToken)
555556
.ConfigureAwait(false);
556557
}
557558
}

projects/RabbitMQ.Client/client/impl/AutorecoveringConnection.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,11 @@ public event EventHandler<RecoveringConsumerEventArgs> RecoveringConsumer
175175

176176
public IProtocol Protocol => Endpoint.Protocol;
177177

178-
public async ValueTask<RecoveryAwareChannel> CreateNonRecoveringChannelAsync()
178+
public async ValueTask<RecoveryAwareChannel> CreateNonRecoveringChannelAsync(CancellationToken cancellationToken)
179179
{
180180
ISession session = InnerConnection.CreateSession();
181181
var result = new RecoveryAwareChannel(_config, session);
182-
return await result.OpenAsync()
182+
return await result.OpenAsync(cancellationToken)
183183
.ConfigureAwait(false) as RecoveryAwareChannel;
184184
}
185185

@@ -241,13 +241,13 @@ await CloseInnerConnectionAsync()
241241
}
242242
}
243243

244-
public async Task<IChannel> CreateChannelAsync()
244+
public async Task<IChannel> CreateChannelAsync(CancellationToken cancellationToken = default)
245245
{
246246
EnsureIsOpen();
247-
RecoveryAwareChannel recoveryAwareChannel = await CreateNonRecoveringChannelAsync()
247+
RecoveryAwareChannel recoveryAwareChannel = await CreateNonRecoveringChannelAsync(cancellationToken)
248248
.ConfigureAwait(false);
249249
AutorecoveringChannel channel = new AutorecoveringChannel(this, recoveryAwareChannel);
250-
await RecordChannelAsync(channel, channelsSemaphoreHeld: false)
250+
await RecordChannelAsync(channel, channelsSemaphoreHeld: false, cancellationToken: cancellationToken)
251251
.ConfigureAwait(false);
252252
return channel;
253253
}

projects/RabbitMQ.Client/client/impl/ChannelBase.cs

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ internal abstract class ChannelBase : IChannel, IRecoverable
5959
private readonly RpcContinuationQueue _continuationQueue = new RpcContinuationQueue();
6060
private readonly ManualResetEventSlim _flowControlBlock = new ManualResetEventSlim(true);
6161

62-
private readonly object _confirmLock = new object();
62+
private object _confirmLock;
6363
private readonly LinkedList<ulong> _pendingDeliveryTags = new LinkedList<ulong>();
6464

6565
private bool _onlyAcksReceived = true;
@@ -183,7 +183,6 @@ public IBasicConsumer DefaultConsumer
183183

184184
public bool IsOpen => CloseReason is null;
185185

186-
// TODO add private bool for Confirm mode
187186
public ulong NextPublishSeqNo { get; private set; }
188187

189188
public string CurrentQueue { get; private set; }
@@ -376,19 +375,22 @@ protected bool Enqueue(IRpcContinuation k)
376375
}
377376
}
378377

379-
internal async Task<IChannel> OpenAsync()
378+
internal async Task<IChannel> OpenAsync(CancellationToken cancellationToken)
380379
{
381380
bool enqueued = false;
382381
var k = new ChannelOpenAsyncRpcContinuation(ContinuationTimeout);
383382

384-
await _rpcSemaphore.WaitAsync(k.CancellationToken)
383+
using CancellationTokenSource lts =
384+
CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, k.CancellationToken);
385+
386+
await _rpcSemaphore.WaitAsync(lts.Token)
385387
.ConfigureAwait(false);
386388
try
387389
{
388390
enqueued = Enqueue(k);
389391

390392
var method = new ChannelOpen();
391-
await ModelSendAsync(method, k.CancellationToken)
393+
await ModelSendAsync(method, lts.Token)
392394
.ConfigureAwait(false);
393395

394396
bool result = await k;
@@ -416,6 +418,8 @@ internal void FinishClose()
416418
m_connectionStartCell?.TrySetResult(null);
417419
}
418420

421+
private bool ConfirmsAreEnabled => _confirmLock != null;
422+
419423
private async Task HandleCommandAsync(IncomingCommand cmd, CancellationToken cancellationToken)
420424
{
421425
/*
@@ -475,17 +479,21 @@ private void OnChannelShutdown(ShutdownEventArgs reason)
475479
{
476480
_continuationQueue.HandleChannelShutdown(reason);
477481
_channelShutdownWrapper.Invoke(this, reason);
478-
lock (_confirmLock)
482+
483+
if (ConfirmsAreEnabled)
479484
{
480-
if (_confirmsTaskCompletionSources?.Count > 0)
485+
lock (_confirmLock)
481486
{
482-
var exception = new AlreadyClosedException(reason);
483-
foreach (var confirmsTaskCompletionSource in _confirmsTaskCompletionSources)
487+
if (_confirmsTaskCompletionSources?.Count > 0)
484488
{
485-
confirmsTaskCompletionSource.TrySetException(exception);
486-
}
489+
var exception = new AlreadyClosedException(reason);
490+
foreach (var confirmsTaskCompletionSource in _confirmsTaskCompletionSources)
491+
{
492+
confirmsTaskCompletionSource.TrySetException(exception);
493+
}
487494

488-
_confirmsTaskCompletionSources.Clear();
495+
_confirmsTaskCompletionSources.Clear();
496+
}
489497
}
490498
}
491499

@@ -581,7 +589,7 @@ protected void HandleBasicNack(in IncomingCommand cmd)
581589
protected void HandleAckNack(ulong deliveryTag, bool multiple, bool isNack)
582590
{
583591
// No need to do this if publisher confirms have never been enabled.
584-
if (NextPublishSeqNo > 0)
592+
if (ConfirmsAreEnabled)
585593
{
586594
// let's take a lock so we can assume that deliveryTags are unique, never duplicated and always sorted
587595
lock (_confirmLock)
@@ -1017,7 +1025,7 @@ await ModelSendAsync(method, k.CancellationToken)
10171025
public async ValueTask BasicPublishAsync<TProperties>(string exchange, string routingKey, TProperties basicProperties, ReadOnlyMemory<byte> body, bool mandatory)
10181026
where TProperties : IReadOnlyBasicProperties, IAmqpHeader
10191027
{
1020-
if (NextPublishSeqNo > 0)
1028+
if (ConfirmsAreEnabled)
10211029
{
10221030
lock (_confirmLock)
10231031
{
@@ -1047,7 +1055,7 @@ public async ValueTask BasicPublishAsync<TProperties>(string exchange, string ro
10471055
}
10481056
catch
10491057
{
1050-
if (NextPublishSeqNo > 0)
1058+
if (ConfirmsAreEnabled)
10511059
{
10521060
lock (_confirmLock)
10531061
{
@@ -1078,7 +1086,7 @@ public async void BasicPublish<TProperties>(CachedString exchange, CachedString
10781086
TProperties basicProperties, ReadOnlyMemory<byte> body, bool mandatory)
10791087
where TProperties : IReadOnlyBasicProperties, IAmqpHeader
10801088
{
1081-
if (NextPublishSeqNo > 0)
1089+
if (ConfirmsAreEnabled)
10821090
{
10831091
lock (_confirmLock)
10841092
{
@@ -1109,7 +1117,7 @@ public async void BasicPublish<TProperties>(CachedString exchange, CachedString
11091117
}
11101118
catch
11111119
{
1112-
if (NextPublishSeqNo > 0)
1120+
if (ConfirmsAreEnabled)
11131121
{
11141122
lock (_confirmLock)
11151123
{
@@ -1126,7 +1134,7 @@ public async ValueTask BasicPublishAsync<TProperties>(CachedString exchange, Cac
11261134
TProperties basicProperties, ReadOnlyMemory<byte> body, bool mandatory)
11271135
where TProperties : IReadOnlyBasicProperties, IAmqpHeader
11281136
{
1129-
if (NextPublishSeqNo > 0)
1137+
if (ConfirmsAreEnabled)
11301138
{
11311139
lock (_confirmLock)
11321140
{
@@ -1157,7 +1165,7 @@ public async ValueTask BasicPublishAsync<TProperties>(CachedString exchange, Cac
11571165
}
11581166
catch
11591167
{
1160-
if (NextPublishSeqNo > 0)
1168+
if (ConfirmsAreEnabled)
11611169
{
11621170
lock (_confirmLock)
11631171
{
@@ -1263,6 +1271,10 @@ await ModelSendAsync(method, k.CancellationToken)
12631271
bool result = await k;
12641272
Debug.Assert(result);
12651273

1274+
// Note:
1275+
// Non-null means confirms are enabled
1276+
_confirmLock = new object();
1277+
12661278
return;
12671279
}
12681280
finally
@@ -1747,7 +1759,7 @@ await ModelSendAsync(method, k.CancellationToken)
17471759

17481760
public Task<bool> WaitForConfirmsAsync(CancellationToken token = default)
17491761
{
1750-
if (NextPublishSeqNo == 0UL)
1762+
if (false == ConfirmsAreEnabled)
17511763
{
17521764
throw new InvalidOperationException("Confirms not selected");
17531765
}
@@ -1821,17 +1833,15 @@ public async Task WaitForConfirmsOrDieAsync(CancellationToken token = default)
18211833
await CloseAsync(ea, false)
18221834
.ConfigureAwait(false);
18231835
}
1824-
catch (TaskCanceledException)
1836+
catch (OperationCanceledException ex)
18251837
{
18261838
const string msg = "timed out waiting for acks";
1827-
1828-
var ex = new IOException(msg);
18291839
var ea = new ShutdownEventArgs(ShutdownInitiator.Library, Constants.ReplySuccess, msg, ex);
18301840

18311841
await CloseAsync(ea, false)
18321842
.ConfigureAwait(false);
18331843

1834-
throw ex;
1844+
throw;
18351845
}
18361846
}
18371847

projects/RabbitMQ.Client/client/impl/Connection.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,12 +256,12 @@ await CloseAsync(ea, true,
256256
}
257257
}
258258

259-
public Task<IChannel> CreateChannelAsync()
259+
public Task<IChannel> CreateChannelAsync(CancellationToken cancellationToken = default)
260260
{
261261
EnsureIsOpen();
262262
ISession session = CreateSession();
263263
var channel = new Channel(_config, session);
264-
return channel.OpenAsync();
264+
return channel.OpenAsync(cancellationToken);
265265
}
266266

267267
internal ISession CreateSession()
@@ -285,7 +285,8 @@ internal void EnsureIsOpen()
285285
}
286286

287287
///<summary>Asynchronous API-side invocation of connection.close with timeout.</summary>
288-
public Task CloseAsync(ushort reasonCode, string reasonText, TimeSpan timeout, bool abort, CancellationToken cancellationToken)
288+
public Task CloseAsync(ushort reasonCode, string reasonText, TimeSpan timeout, bool abort,
289+
CancellationToken cancellationToken = default)
289290
{
290291
var reason = new ShutdownEventArgs(ShutdownInitiator.Application, reasonCode, reasonText);
291292
return CloseAsync(reason, abort, timeout, cancellationToken);

projects/RabbitMQ.Client/client/impl/Frame.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,8 +313,14 @@ internal static bool TryReadFrame(ref ReadOnlySequence<byte> buffer, uint maxMes
313313
return false;
314314
}
315315

316-
// TODO check this?
317-
// buffer.IsSingleSegment;
316+
/*
317+
* Note:
318+
* The use of buffer.Slice seems to take all segments into account, thus there appears to be no need to check IsSingleSegment
319+
* Debug.Assert(buffer.IsSingleSegment);
320+
* In addition, the TestBasicRoundtripConcurrentManyMessages asserts that the consumed message bodies are equivalent to
321+
* the published bodies, and if there were an issue parsing frames, it would show up there for sure.
322+
* https://github.com/rabbitmq/rabbitmq-dotnet-client/pull/1516#issuecomment-1991943017
323+
*/
318324

319325
byte firstByte = buffer.First.Span[0];
320326
if (firstByte == 'A')

projects/RabbitMQ.Client/client/impl/SocketFrameHandler.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,6 @@ await tcpClient.ConnectAsync(endpoint.Address, endpoint.Port, linkedTokenSource.
428428
{
429429
if (timeoutTokenSource.Token.IsCancellationRequested)
430430
{
431-
// TODO maybe do not use System.TimeoutException here
432431
var timeoutException = new TimeoutException(msg, e);
433432
throw new ConnectFailureException(msg, timeoutException);
434433
}

0 commit comments

Comments
 (0)