Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix EventLoopFuture.and's serious threading issues [alternative] #176

Merged
merged 1 commit into from
Mar 16, 2018

Conversation

weissi
Copy link
Member

@weissi weissi commented Mar 16, 2018

alternative to #175

Motivation:

EventLoopFuture.and had serious threading issues if the EventLoops
weren't the same.

Modifications:

Fixed the threading issues and tested them properly.

Result:

Hopefully and and andAll now don't crash if you use them across
EventLoops.

Motivation:

EventLoopFuture.and had serious threading issues if the EventLoops
weren't the same.

Modifications:

Fixed the threading issues and tested them properly.

Result:

Hopefully `and` and `andAll` now don't crash if you use them across
EventLoops.
@weissi weissi force-pushed the jw-fix-future-and-threading-alt branch from 18fafdb to ab05365 Compare March 16, 2018 11:11
Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Ok, I think I like this a lot better than its competitor. We should take this patch, and then add a work item to go back and revisit the APIs here.

@weissi
Copy link
Member Author

weissi commented Mar 16, 2018

thanks for pushing me to do something @Lukasa , thought the alternatives would be too risky (in perf/bugs) but wasn't quite right ;)

_whenComplete { () -> CallbackList in
switch self.value! {
case .failure(let error):
return promise._setValue(value: .failure(error))
case .success(let t):
andlock.lock()
Copy link
Member

Choose a reason for hiding this comment

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

less locks FTW!

@normanmaurer normanmaurer added this to the 1.3.0 milestone Mar 16, 2018
@normanmaurer normanmaurer merged commit 320561f into apple:master Mar 16, 2018
@normanmaurer normanmaurer added the 🔨 semver/patch No public API change. label Mar 16, 2018
@Lukasa Lukasa modified the milestones: 1.3.0, 1.2.1 Mar 16, 2018
tiagomartinho pushed a commit to tiagomartinho/swift-nio that referenced this pull request Mar 17, 2018
Motivation:

EventLoopFuture.and had serious threading issues if the EventLoops
weren't the same.

Modifications:

Fixed the threading issues and tested them properly.

Result:

Hopefully `and` and `andAll` now don't crash if you use them across
EventLoops.
weissi pushed a commit to weissi/swift-nio that referenced this pull request Jun 13, 2020
Motivation:

The code-of-conduct email address is out-of-date.

Modifications:

Update code-of-conduct email address to swift-server-conduct@group.apple.com

Result:

- Code of conduct email address is up-to-date.
weissi pushed a commit to weissi/swift-nio that referenced this pull request Feb 3, 2024
Motivation

Currently we don't confirm that the decompression has completed
successfully. This means that we can incorrectly spin forever attempting
to decompress past the end of a message, and that we can fail to notice
that a message is truncated. Neither of these is good.

Modifications

Propagate the message zlib gives us as to whether or not decompression
is done, and keep track of it.
Add some tests written by @vojtarylko to validate the behaviour.

Result

Correctly police the bounds of the messages.
Resolves apple#175 and apple#176.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants