Skip to content

Commit 29545cc

Browse files
committed
bugfix: prevent mutating susbcriptions when unsubscribing
- If the reference to the subscriptions list is passed in directly, prevent ``unsubscribe`` from mutating the list by re-creating the list before iterating over it to unsubscribe. - Add a test to verify that the bug is fixed.
1 parent 0b6239f commit 29545cc

File tree

2 files changed

+32
-2
lines changed

2 files changed

+32
-2
lines changed

tests/core/subscriptions/test_subscription_manager.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,3 +188,28 @@ async def test_unsubscribe_with_one_or_multiple(subscription_manager):
188188
# unsubscribe non-existent subscription object
189189
with pytest.raises(Web3ValueError, match=f"Subscription not found|{sub5.id}"):
190190
await subscription_manager.unsubscribe(sub5)
191+
192+
193+
@pytest.mark.asyncio
194+
async def test_unsubscribe_with_subscriptions_reference_does_not_mutate_the_list(
195+
subscription_manager,
196+
):
197+
sub1 = NewHeadsSubscription()
198+
sub2 = LogsSubscription()
199+
sub3 = PendingTxSubscription()
200+
sub4 = NewHeadsSubscription()
201+
202+
await subscription_manager.subscribe([sub1, sub2, sub3, sub4])
203+
assert subscription_manager.subscriptions == [sub1, sub2, sub3, sub4]
204+
205+
# assert not mutating in place
206+
await subscription_manager.unsubscribe(subscription_manager.subscriptions)
207+
assert subscription_manager.subscriptions == []
208+
209+
# via unsubscribe all
210+
211+
await subscription_manager.subscribe([sub1, sub2, sub3, sub4])
212+
assert subscription_manager.subscriptions == [sub1, sub2, sub3, sub4]
213+
214+
await subscription_manager.unsubscribe_all()
215+
assert subscription_manager.subscriptions == []

web3/providers/persistent/subscription_manager.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,10 @@ async def unsubscribe(
206206
raise Web3ValueError("No subscriptions provided.")
207207

208208
unsubscribed: List[bool] = []
209-
for sub in subscriptions:
209+
# re-create the subscription list to prevent modifying the original list
210+
# in case ``subscription_manager.subscriptions`` was passed in directly
211+
subs = [sub for sub in subscriptions]
212+
for sub in subs:
210213
if isinstance(sub, str):
211214
sub = HexStr(sub)
212215
unsubscribed.append(await self.unsubscribe(sub))
@@ -226,7 +229,9 @@ async def unsubscribe_all(self) -> bool:
226229
:rtype: bool
227230
"""
228231
unsubscribed = [
229-
await self.unsubscribe(sub) for sub in self.subscriptions.copy()
232+
await self.unsubscribe(sub)
233+
# use copy to prevent modifying the list while iterating over it
234+
for sub in self.subscriptions.copy()
230235
]
231236
if all(unsubscribed):
232237
self.logger.info("Successfully unsubscribed from all subscriptions.")

0 commit comments

Comments
 (0)