Skip to content

Fix: race conditions in DefaultNetworkService.lookup by removing async from AttachmentAllocator.lookup#1699

Open
GarySkywalker-droid wants to merge 3 commits into
apple:mainfrom
GarySkywalker-droid:fix/dns-racing-condition
Open

Fix: race conditions in DefaultNetworkService.lookup by removing async from AttachmentAllocator.lookup#1699
GarySkywalker-droid wants to merge 3 commits into
apple:mainfrom
GarySkywalker-droid:fix/dns-racing-condition

Conversation

@GarySkywalker-droid

@GarySkywalker-droid GarySkywalker-droid commented Jun 11, 2026

Copy link
Copy Markdown

Type of Change

  • [*] Bug fix
  • New feature
  • Breaking change
  • Documentation update

Motivation and Context

Removed the async from AttachmentAllocator.lookup, causing the race conditions because the deallocator being called while lookup is running as it overrides the actor's reentrancy protection. Leading to at times mac address being returned as nil and fixes the #1693

Testing

  • Tested locally
  • Added/updated tests
  • Added/updated docs

…ace conditions because the deallocator being called while lookup is running as it overrides the actor's reentrancy protection
@jglogan jglogan requested a review from katiewasnothere June 15, 2026 17:19
@katiewasnothere

Copy link
Copy Markdown
Contributor

Hi @GarySkywalker-droid could you describe the race condition this change fixes a bit more? I'm not clear where the issue is

@kiwigitops kiwigitops left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this still needs an await at the call site. Even if lookup(hostname:) is no longer declared async, AttachmentAllocator is an actor, and this call is still crossing from DefaultNetworkService to a different actor instance. Swift treats that as an asynchronous actor-isolated access, so try allocator.lookup(hostname:) should not compile as written.

That also means this change may not remove the suspension point the PR is trying to avoid. The race probably needs a different shape, such as performing the hostname lookup inside the allocator operation that owns the related state transition, or otherwise avoiding a separate cross-actor read.

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.

5 participants