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

Improve pointer hygeine in SocketAddresses.swift #1588

Merged
merged 1 commit into from
Jul 10, 2020

Conversation

Lukasa
Copy link
Contributor

@Lukasa Lukasa commented Jul 9, 2020

Motivation:

The socket address types are some of the worst types for handling in
Swift. They pervasively type-pun in a way that Swift strongly wants to
resist, and they expose a bunch of data through awkward fixed-length
arrays.

We've done a number of passes over this code to improve its pointer
hygeine. This is another one. Here, we remove some uses of the memory
binding APIs that were unnecessary, and that can instead take advantage
of the way Swift handles binding memory of homogeneous tuple types.

In Swift, whenever we have a homogeneous tuple (i.e. a tuple whose
elements are all the same type), that memory is actually bound to two
types: to the tuple type, and to the type of the elements. This allows
us to use assumingMemoryBound(to:) to convert between the pointer to
the tuple and the pointer to the first element of the tuple.

This knowledge lets us remove calls to bindMemory and
withMemoryRebound. Neither call is correct to use in this situation,
and while we were unlikely to encounter any damage, this is the best
possible use of the Swift pointer APIs for this work.

Modifications:

  • Rewrite accesses to the UNIX domain socket path to use appropriately
    typed pointers without binding or rebinding memory.
  • Added some assertions about the static types of the stored elements.

Result:

Better management of pointer type bindings.

@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Jul 9, 2020
@Lukasa Lukasa added this to the 2.20.0 milestone Jul 9, 2020
Motivation:

The socket address types are some of the worst types for handling in
Swift. They pervasively type-pun in a way that Swift strongly wants to
resist, and they expose a bunch of data through awkward fixed-length
arrays.

We've done a number of passes over this code to improve its pointer
hygeine. This is another one. Here, we remove some uses of the memory
binding APIs that were unnecessary, and that can instead take advantage
of the way Swift handles binding memory of homogeneous tuple types.

In Swift, whenever we have a homogeneous tuple (i.e. a tuple whose
elements are all the same type), that memory is actually bound to _two_
types: to the tuple type, and to the type of the elements. This allows
us to use `assumingMemoryBound(to:)` to convert between the pointer to
the tuple and the pointer to the first element of the tuple.

This knowledge lets us remove calls to `bindMemory` and
`withMemoryRebound`. Neither call is correct to use in this situation,
and while we were unlikely to encounter any damage, this is the best
possible use of the Swift pointer APIs for this work.

Modifications:

- Rewrite accesses to the UNIX domain socket path to use appropriately
  typed pointers without binding or rebinding memory.
- Added some assertions about the static types of the stored elements.

Result:

Better management of pointer type bindings.
@Lukasa Lukasa force-pushed the cb-shuffle-the-deck-chairs branch from 135cb13 to dab15b9 Compare July 9, 2020 16:25
Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for useful motivation + comments!

Copy link
Contributor

@PeterAdams-A PeterAdams-A left a comment

Choose a reason for hiding this comment

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

Yes - agree this is an improvement over previous.

@Lukasa Lukasa merged commit 9745013 into apple:master Jul 10, 2020
slashmo pushed a commit to slashmo/swift-nio that referenced this pull request Aug 18, 2020
Motivation:

The socket address types are some of the worst types for handling in
Swift. They pervasively type-pun in a way that Swift strongly wants to
resist, and they expose a bunch of data through awkward fixed-length
arrays.

We've done a number of passes over this code to improve its pointer
hygeine. This is another one. Here, we remove some uses of the memory
binding APIs that were unnecessary, and that can instead take advantage
of the way Swift handles binding memory of homogeneous tuple types.

In Swift, whenever we have a homogeneous tuple (i.e. a tuple whose
elements are all the same type), that memory is actually bound to _two_
types: to the tuple type, and to the type of the elements. This allows
us to use `assumingMemoryBound(to:)` to convert between the pointer to
the tuple and the pointer to the first element of the tuple.

This knowledge lets us remove calls to `bindMemory` and
`withMemoryRebound`. Neither call is correct to use in this situation,
and while we were unlikely to encounter any damage, this is the best
possible use of the Swift pointer APIs for this work.

Modifications:

- Rewrite accesses to the UNIX domain socket path to use appropriately
  typed pointers without binding or rebinding memory.
- Added some assertions about the static types of the stored elements.

Result:

Better management of pointer type bindings.
# 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.

4 participants