Skip to content

Commit 88bfbcb

Browse files
KaoticzKaoticz
authored
No more NREs when disposing/finalizing, fixes #1547 (#1567)
* Fixed NREs when disposing/finalizing.. * Some additional safeguards, just in case * Prevent disposed sharded clients from throwing InvalidOperationException in the GC
1 parent 10019d2 commit 88bfbcb

File tree

19 files changed

+162
-178
lines changed

19 files changed

+162
-178
lines changed

‎DSharpPlus.CommandsNext/CommandsNextExtension.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -166,11 +166,11 @@ internal CommandsNextExtension(CommandsNextConfiguration cfg)
166166
/// Disposes of this the resources used by CNext.
167167
/// </summary>
168168
public override void Dispose()
169-
=> this.Config.CommandExecutor.Dispose();
170-
171-
~CommandsNextExtension()
172169
{
173-
this.Dispose();
170+
this.Config.CommandExecutor.Dispose();
171+
172+
// Satisfy rule CA1816. Can be removed if this class is sealed.
173+
GC.SuppressFinalize(this);
174174
}
175175

176176
#region DiscordClient Registration

‎DSharpPlus.Interactivity/EventHandling/EventWaiter.cs‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -133,31 +133,33 @@ private Task HandleEvent(DiscordClient client, T eventargs)
133133
return Task.CompletedTask;
134134
}
135135

136-
~EventWaiter()
137-
{
138-
this.Dispose();
139-
}
140-
141136
/// <summary>
142137
/// Disposes this EventWaiter
143138
/// </summary>
144139
public void Dispose()
145140
{
141+
if (this._disposed)
142+
return;
143+
146144
this._disposed = true;
147-
if (this._event != null)
145+
146+
if (this._event != null && this._handler != null)
148147
this._event.Unregister(this._handler);
149148

150-
this._event = null;
151-
this._handler = null;
152-
this._client = null;
149+
this._event = null!;
150+
this._handler = null!;
151+
this._client = null!;
153152

154153
if (this._matchrequests != null)
155154
this._matchrequests.Clear();
156155
if (this._collectrequests != null)
157156
this._collectrequests.Clear();
158157

159-
this._matchrequests = null;
160-
this._collectrequests = null;
158+
this._matchrequests = null!;
159+
this._collectrequests = null!;
160+
161+
// Satisfy rule CA1816. Can be removed if this class is sealed.
162+
GC.SuppressFinalize(this);
161163
}
162164
}
163165
}

‎DSharpPlus.Interactivity/EventHandling/Paginator.cs‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -260,22 +260,23 @@ private async Task PaginateAsync(IPaginationRequest p, DiscordEmoji emoji)
260260
await builder.ModifyAsync(msg);
261261
}
262262

263-
~Paginator()
264-
{
265-
this.Dispose();
266-
}
267-
268263
/// <summary>
269264
/// Disposes this EventWaiter
270265
/// </summary>
271266
public void Dispose()
272267
{
273-
this._client.MessageReactionAdded -= this.HandleReactionAdd;
274-
this._client.MessageReactionRemoved -= this.HandleReactionRemove;
275-
this._client.MessageReactionsCleared -= this.HandleReactionClear;
276-
this._client = null;
277-
this._requests.Clear();
278-
this._requests = null;
268+
// Why doesn't this class implement IDisposable?
269+
270+
if (this._client != null)
271+
{
272+
this._client.MessageReactionAdded -= this.HandleReactionAdd;
273+
this._client.MessageReactionRemoved -= this.HandleReactionRemove;
274+
this._client.MessageReactionsCleared -= this.HandleReactionClear;
275+
this._client = null!;
276+
}
277+
278+
this._requests?.Clear();
279+
this._requests = null!;
279280
}
280281
}
281282
}

‎DSharpPlus.Interactivity/EventHandling/Poller.cs‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -128,22 +128,26 @@ private Task HandleReactionClear(DiscordClient client, MessageReactionsClearEven
128128
return Task.CompletedTask;
129129
}
130130

131-
~Poller()
132-
{
133-
this.Dispose();
134-
}
135-
136131
/// <summary>
137132
/// Disposes this EventWaiter
138133
/// </summary>
139134
public void Dispose()
140135
{
141-
this._client.MessageReactionAdded -= this.HandleReactionAdd;
142-
this._client.MessageReactionRemoved -= this.HandleReactionRemove;
143-
this._client.MessageReactionsCleared -= this.HandleReactionClear;
144-
this._client = null;
145-
this._requests.Clear();
146-
this._requests = null;
136+
// Why doesn't this class implement IDisposable?
137+
138+
if (this._client != null)
139+
{
140+
this._client.MessageReactionAdded -= this.HandleReactionAdd;
141+
this._client.MessageReactionRemoved -= this.HandleReactionRemove;
142+
this._client.MessageReactionsCleared -= this.HandleReactionClear;
143+
this._client = null!;
144+
}
145+
146+
if (this._requests != null)
147+
{
148+
this._requests.Clear();
149+
this._requests = null!;
150+
}
147151
}
148152
}
149153
}

‎DSharpPlus.Interactivity/EventHandling/ReactionCollector.cs‎

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -168,31 +168,34 @@ private Task HandleReactionClear(DiscordClient client, MessageReactionsClearEven
168168
return Task.CompletedTask;
169169
}
170170

171-
~ReactionCollector()
172-
{
173-
this.Dispose();
174-
}
175-
176171
/// <summary>
177172
/// Disposes this EventWaiter
178173
/// </summary>
179174
public void Dispose()
180175
{
181-
this._client = null;
176+
this._client = null!;
177+
178+
if (this._reactionAddHandler != null)
179+
this._reactionAddEvent?.Unregister(this._reactionAddHandler);
182180

183-
this._reactionAddEvent.Unregister(this._reactionAddHandler);
184-
this._reactionRemoveEvent.Unregister(this._reactionRemoveHandler);
185-
this._reactionClearEvent.Unregister(this._reactionClearHandler);
181+
if (this._reactionRemoveHandler != null)
182+
this._reactionRemoveEvent?.Unregister(this._reactionRemoveHandler);
186183

187-
this._reactionAddEvent = null;
188-
this._reactionAddHandler = null;
189-
this._reactionRemoveEvent = null;
190-
this._reactionRemoveHandler = null;
191-
this._reactionClearEvent = null;
192-
this._reactionClearHandler = null;
184+
if (this._reactionClearHandler != null)
185+
this._reactionClearEvent?.Unregister(this._reactionClearHandler);
193186

194-
this._requests.Clear();
195-
this._requests = null;
187+
this._reactionAddEvent = null!;
188+
this._reactionAddHandler = null!;
189+
this._reactionRemoveEvent = null!;
190+
this._reactionRemoveHandler = null!;
191+
this._reactionClearEvent = null!;
192+
this._reactionClearHandler = null!;
193+
194+
this._requests?.Clear();
195+
this._requests = null!;
196+
197+
// Satisfy rule CA1816. Can be removed if this class is sealed.
198+
GC.SuppressFinalize(this);
196199
}
197200
}
198201

@@ -214,19 +217,16 @@ public ReactionCollectRequest(DiscordMessage msg, TimeSpan timeout)
214217
this._ct.Token.Register(() => this._tcs.TrySetResult(null));
215218
}
216219

217-
~ReactionCollectRequest()
218-
{
219-
this.Dispose();
220-
}
221-
222220
public void Dispose()
223221
{
224-
GC.SuppressFinalize(this);
225222
this._ct.Dispose();
226223
this._tcs = null;
227224
this._message = null;
228225
this._collected?.Clear();
229226
this._collected = null;
227+
228+
// Satisfy rule CA1816. Can be removed if this class is sealed.
229+
GC.SuppressFinalize(this);
230230
}
231231
}
232232

‎DSharpPlus.Interactivity/EventHandling/Requests/CollectRequest.cs‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,24 +57,19 @@ public CollectRequest(Func<T, bool> predicate, TimeSpan timeout)
5757
this._collected = new ConcurrentHashSet<T>();
5858
}
5959

60-
~CollectRequest()
61-
{
62-
this.Dispose();
63-
}
64-
6560
/// <summary>
6661
/// Disposes this CollectRequest.
6762
/// </summary>
6863
public void Dispose()
6964
{
7065
this._ct.Dispose();
71-
this._tcs = null;
72-
this._predicate = null;
66+
this._tcs = null!;
67+
this._predicate = null!;
7368

7469
if (this._collected != null)
7570
{
7671
this._collected.Clear();
77-
this._collected = null;
72+
this._collected = null!;
7873
}
7974
}
8075
}

‎DSharpPlus.Interactivity/EventHandling/Requests/MatchRequest.cs‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,19 +54,17 @@ public MatchRequest(Func<T, bool> predicate, TimeSpan timeout)
5454
this._timeout = timeout;
5555
}
5656

57-
~MatchRequest()
58-
{
59-
this.Dispose();
60-
}
61-
6257
/// <summary>
6358
/// Disposes this MatchRequest.
6459
/// </summary>
6560
public void Dispose()
6661
{
67-
this._ct.Dispose();
68-
this._tcs = null;
69-
this._predicate = null;
62+
this._ct?.Dispose();
63+
this._tcs = null!;
64+
this._predicate = null!;
65+
66+
// Satisfy rule CA1816. Can be removed if this class is sealed.
67+
GC.SuppressFinalize(this);
7068
}
7169
}
7270
}

‎DSharpPlus.Interactivity/EventHandling/Requests/PaginationRequest.cs‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -195,18 +195,15 @@ public async Task<TaskCompletionSource<bool>> GetTaskCompletionSourceAsync()
195195
return this._tcs;
196196
}
197197

198-
~PaginationRequest()
199-
{
200-
this.Dispose();
201-
}
202-
203198
/// <summary>
204199
/// Disposes this PaginationRequest.
205200
/// </summary>
206201
public void Dispose()
207202
{
208-
this._ct.Dispose();
209-
this._tcs = null;
203+
// Why doesn't this class implement IDisposable?
204+
205+
this._ct?.Dispose();
206+
this._tcs = null!;
210207
}
211208
}
212209
}

‎DSharpPlus.Interactivity/EventHandling/Requests/PollRequest.cs‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,18 +99,15 @@ internal void AddReaction(DiscordEmoji emoji, DiscordUser member)
9999
}
100100
}
101101

102-
~PollRequest()
103-
{
104-
this.Dispose();
105-
}
106-
107102
/// <summary>
108103
/// Disposes this PollRequest.
109104
/// </summary>
110105
public void Dispose()
111106
{
112-
this._ct.Dispose();
113-
this._tcs = null;
107+
// Why doesn't this class implement IDisposable?
108+
109+
this._ct?.Dispose();
110+
this._tcs = null!;
114111
}
115112
}
116113

‎DSharpPlus.Interactivity/InteractivityExtension.cs‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,21 +1060,19 @@ private async Task HandleInvalidInteraction(DiscordInteraction interaction)
10601060

10611061
public override void Dispose()
10621062
{
1063-
this.ComponentEventWaiter.Dispose();
1064-
this.ModalEventWaiter.Dispose();
1065-
this.ReactionCollector.Dispose();
1066-
this.ComponentInteractionWaiter.Dispose();
1067-
this.MessageCreatedWaiter.Dispose();
1068-
this.MessageReactionAddWaiter.Dispose();
1069-
this.Paginator.Dispose();
1070-
this.Poller.Dispose();
1071-
this.TypingStartWaiter.Dispose();
1072-
this._compPaginator.Dispose();
1073-
}
1074-
1075-
~InteractivityExtension()
1076-
{
1077-
this.Dispose();
1063+
this.ComponentEventWaiter?.Dispose();
1064+
this.ModalEventWaiter?.Dispose();
1065+
this.ReactionCollector?.Dispose();
1066+
this.ComponentInteractionWaiter?.Dispose();
1067+
this.MessageCreatedWaiter?.Dispose();
1068+
this.MessageReactionAddWaiter?.Dispose();
1069+
this.Paginator?.Dispose();
1070+
this.Poller?.Dispose();
1071+
this.TypingStartWaiter?.Dispose();
1072+
this._compPaginator?.Dispose();
1073+
1074+
// Satisfy rule CA1816. Can be removed if this class is sealed.
1075+
GC.SuppressFinalize(this);
10781076
}
10791077
}
10801078
}

0 commit comments

Comments
 (0)