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.

Sign up for GitHub

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

Jump to bottom

ChannelInvoker.newPromise should pass file and line to eventLoop.newPromise #202

Closed
Lukasa opened this issue Mar 20, 2018 · 1 comment · Fixed by #208
Closed

ChannelInvoker.newPromise should pass file and line to eventLoop.newPromise #202

Lukasa opened this issue Mar 20, 2018 · 1 comment · Fixed by #208
Labels
good first issue Good for newcomers kind/bug Feature doesn't work as expected.
Milestone
1.3.0

Comments

@Lukasa
Copy link
Contributor

Lukasa commented Mar 20, 2018

We normally want debugging information to keep track of where our promises are allocated in debug modes. However, if someone does Channel.newPromise instead of eventLoop.newPromise, that information is lost. The core issue is here:

swift-nio/Sources/NIO/ChannelInvoker.swift

Lines 182 to 184 in fef728c

private func newPromise() -> EventLoopPromise<Void> {
return eventLoop.newPromise()
}

This indirection should preserve the #file and #line specific debugging logic we use elsewhere.
@Lukasa Lukasa added the kind/bug Feature doesn't work as expected. label Mar 20, 2018
@weissi weissi added the good first issue Good for newcomers label Mar 20, 2018
@tib tib mentioned this issue Mar 21, 2018
Merged
@tib
Copy link
Contributor

tib commented Mar 21, 2018

Hey guys, I think I've fixed this with my PR.

normanmaurer pushed a commit that referenced this issue Mar 21, 2018
@tib @normanmaurer
ChannelInvoker.newPromise should pass file and line to eventLoop.newP…
9a6f34f 
…romise (#202) (#208)

Motivation:

Introduced some new parameters to fix #202

Modifications:

Created new parameters for the file and line newPromise method inside ChannelOutboundInvoker.

Result:

File and line numbers are now passed to the eventLoop object, all unit tests have been passed.
@normanmaurer normanmaurer added this to the 1.3.0 milestone Mar 21, 2018
weissi pushed a commit to weissi/swift-nio that referenced this issue Jun 13, 2020
@johnkassebaum @Lukasa
Make multiplexer's targetWindowSize configurable and use configurabil…
0ca9705 
…ity when creating stream channels. (apple#202)

Motivation:
Infow target window size constrains throughput. The default hardcoded 65535 is low for some use cases and so needs to be configrable. A TODO in the code stated 'Make configurable'.

Modifications:
- In HTTP2StreamMultiplexer, responding to a TODO where stream multiplexer creates a stream channel, created a multiplexr stored property to be used which is set in the default initilaizer.
- In HTTP2PipelineHelpers, added versions of configureHTTP2Pipeline(..) and configureCommonHTTP2ServerPipeline that require an activeTargetWindow arg.

Result:
Clients, such as GRPCClient, can configure pipelines with targetWindowSizes to their liking.

Co-authored-by: Cory Benfield <lukasa@apple.com>
weissi pushed a commit to weissi/swift-nio that referenced this issue Jun 13, 2020
@Lukasa
Respect target window size on inbound channels. (apple#205)
c8f952d 
Motivation:

As part of apple#202 we added support for configuring the target window size
of stream channels. That was plumbed through into outbound channels, but
we missed the inbound ones, meaning that this only really worked for
clients, not servers. That hardly seems fair!

Modifications:

Pass target window size to inbound channels too.

Result:

We can control target window size of inbound channels.
weissi pushed a commit to weissi/swift-nio that referenced this issue Feb 3, 2024
@guoye-zhang
Add HTTP types adapter for SwiftNIO (apple#202)
798c962 
* Add HTTP types adapter for SwiftNIO

* swiftformat

* Guard on Swift 5.8

* Review comments

* Update swift-http-types to 0.1.1

* Update swift-http-types to 1.0.0

* Review feedback

* Review feedback

* Bump minimum Swift version to 5.7.1

* Allow Host in any order
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers kind/bug Feature doesn't work as expected.
Projects
None yet
Milestone
1.3.0
Development

Successfully merging a pull request may close this issue.

fixed #202
4 participants
@tib @normanmaurer @weissi @Lukasa