Skip to content

NIOThread: remove the detached threads functionality#3304

Merged
weissi merged 1 commit intoapple:mainfrom
weissi:jw-rm-detached
Jul 15, 2025
Merged

NIOThread: remove the detached threads functionality#3304
weissi merged 1 commit intoapple:mainfrom
weissi:jw-rm-detached

Conversation

@weissi
Copy link
Copy Markdown
Member

@weissi weissi commented Jul 15, 2025

Motivation:

Detached threads are a bad idea, their lifetime becomes random and NIO doesn't need this functionality (except for in some tests that are easily refactored)

Modifications:

  • Refactor tests to not use detached threads
  • Remove detached threads functionality

Result:

  • Cleaner, less complex and more correct code

@weissi weissi requested a review from Lukasa July 15, 2025 09:44
@weissi weissi added the 🔨 semver/patch No public API change. label Jul 15, 2025
weissi added a commit that referenced this pull request Jul 15, 2025
### Motivation:

Structured Concurrency is helpful but so far, MTELG was very difficult
to use with it mostly because Swift is still missing `async` in
`defer`s.

This builds on
- #3297 
- #3302
- #3304

### Modifications:

- Add MTELG.withELG { ... }

### Result:

MTELG + SC = <3
internal init(handle: ThreadOpsSystem.ThreadHandle, isDetachedThread: Bool, desiredName: String?) {
self.handle = NIOLockedValueBox(isDetachedThread ? nil : handle)
internal init(handle: ThreadOpsSystem.ThreadHandle, desiredName: String?) {
self.handle = NIOLockedValueBox(handle)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is being optional-promoted, you can remove the optional on the type definition.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Alas when we join() we need to set it to nil to not hold onto the then-deallocated pthread_t.

@weissi weissi requested a review from Lukasa July 15, 2025 13:34
@weissi weissi enabled auto-merge (squash) July 15, 2025 13:55
@weissi weissi merged commit 344db24 into apple:main Jul 15, 2025
41 checks passed
@weissi weissi deleted the jw-rm-detached branch July 15, 2025 14:45
zaneenders pushed a commit to zaneenders/swift-nio that referenced this pull request Jul 23, 2025
### Motivation:

Structured Concurrency is helpful but so far, MTELG was very difficult
to use with it mostly because Swift is still missing `async` in
`defer`s.

This builds on
- apple#3297 
- apple#3302
- apple#3304

### Modifications:

- Add MTELG.withELG { ... }

### Result:

MTELG + SC = <3
zaneenders pushed a commit to zaneenders/swift-nio that referenced this pull request Jul 23, 2025
### Motivation:

Detached threads are a bad idea, their lifetime becomes random and NIO
doesn't need this functionality (except for in some tests that are
easily refactored)

### Modifications:

- Refactor tests to not use detached threads
- Remove detached threads functionality

### Result:

- Cleaner, less complex and more correct code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨 semver/patch No public API change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants