Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

refactor: rewrite accept resources #3271

Merged
merged 6 commits into from Nov 7, 2019

Conversation

@bartlomieju
Copy link
Contributor

bartlomieju commented Nov 5, 2019

No description provided.

cli/ops/tls.rs Outdated Show resolved Hide resolved
@bartlomieju

This comment has been minimized.

Copy link
Contributor Author

bartlomieju commented Nov 5, 2019

CC @ry

cli/ops/net.rs Outdated Show resolved Hide resolved
core/resources.rs Outdated Show resolved Hide resolved
@bartlomieju bartlomieju force-pushed the bartlomieju:refactor-accept_resources branch from dde6757 to 42a91ce Nov 7, 2019
})))
}

#[derive(Debug, PartialEq)]
enum AcceptTlsState {
Eager,

This comment has been minimized.

Copy link
@ry

ry Nov 7, 2019

Collaborator

Given the fact that we do eager polling in core, I wonder if this can be removed?

This comment has been minimized.

Copy link
@bartlomieju

bartlomieju Nov 7, 2019

Author Contributor

Good catch

This comment has been minimized.

Copy link
@bartlomieju

bartlomieju Nov 7, 2019

Author Contributor

Oooops I forgot why it was there... I guess we have to keep it there. If we eager poll we shouldn't track task because it would be associated with that task. Then the future is added to pending futures (because it wasn't ready), it will be picked by by another task and panic that it's already tracked by other task.

impl Drop for TcpListenerResource {
fn drop(&mut self) {
self.notify_task();
}

This comment has been minimized.

Copy link
@ry

ry Nov 7, 2019

Collaborator

👍

@bartlomieju bartlomieju force-pushed the bartlomieju:refactor-accept_resources branch from de769e7 to 42a91ce Nov 7, 2019
@ry
ry approved these changes Nov 7, 2019
Copy link
Collaborator

ry left a comment

LGTM

@ry ry merged commit 415d4c2 into denoland:master Nov 7, 2019
19 checks passed
19 checks passed
test macOS-latest
Details
test macOS-latest
Details
test_std macOS-latest
Details
test_std macOS-latest
Details
test windows-2019
Details
test windows-2019
Details
test_std windows-2019
Details
test_std windows-2019
Details
test ubuntu-16.04
Details
test ubuntu-16.04
Details
test_debug ubuntu-16.04
Details
test_debug ubuntu-16.04
Details
test_std ubuntu-16.04
Details
test_std ubuntu-16.04
Details
bench ubuntu-16.04
Details
bench ubuntu-16.04
Details
lint ubuntu-16.04
Details
lint ubuntu-16.04
Details
license/cla Contributor License Agreement is signed.
Details
@bartlomieju bartlomieju deleted the bartlomieju:refactor-accept_resources branch Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.