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

Ensure localAddress / remoteAddress are still accessible in channelIn… #346

Merged
merged 2 commits into from
Apr 23, 2018

Conversation

normanmaurer
Copy link
Member

…active / handlerRemoved

Motivation:

Often its useful to be still be able to access the local / remote address during channelInactive / handlerRemoved callbacks to for example log it. We should ensure its still accessible during it.

Modifications:

  • Fallback to slow-path in ChannelHandlerContext.localAddress0 / remoteAddress0 if fast-path fails to try accessing the address via the cache.
  • Clear cached addresses after all callbacks are run.
  • Add unit test.

Result:

Be able to access addresses while handlers are notified.

@normanmaurer normanmaurer requested review from Lukasa and weissi April 23, 2018 09:08
@normanmaurer
Copy link
Member Author

@ldewailly PTAL as well

do {
// Fast-path access to the remoteAddress.
return try self.channel._unsafe.remoteAddress0()
} catch let err as ChannelError where err == .ioOnClosedChannel {
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for this verbose syntax, just do catch ChannelError.ioOnClosedChannel.

Copy link
Member

Choose a reason for hiding this comment

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

TIL

do {
// Fast-path access to the localAddress.
return try self.channel._unsafe.localAddress0()
} catch let err as ChannelError where err == .ioOnClosedChannel {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

@normanmaurer normanmaurer force-pushed the local_remote_address_inactive branch 2 times, most recently from 8cf0ced to f960e25 Compare April 23, 2018 09:52
@normanmaurer
Copy link
Member Author

@Lukasa addressed

…active / handlerRemoved

Motivation:

Often its useful to be still be able to access the local / remote address during channelInactive / handlerRemoved callbacks to for example log it. We should ensure its still accessible during it.

Modifications:

- Fallback to slow-path in ChannelHandlerContext.localAddress0 / remoteAddress0 if fast-path fails to try accessing the address via the cache.
- Clear cached addresses after all callbacks are run.
- Add unit test.

Result:

Be able to access addresses while handlers are notified.
@normanmaurer normanmaurer force-pushed the local_remote_address_inactive branch from f960e25 to d655f04 Compare April 23, 2018 10:10
Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

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

lgtm

@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Apr 23, 2018
@Lukasa Lukasa added this to the 1.6.0 milestone Apr 23, 2018
@normanmaurer normanmaurer merged commit 9f01374 into apple:master Apr 23, 2018
@normanmaurer normanmaurer deleted the local_remote_address_inactive branch April 23, 2018 10:51
# 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