Skip to content

Conversation

@LukasWoodtli
Copy link

async recvmsg and sendmsg

Following the discussion here: https://discuss.python.org/t/expanding-asyncio-support-for-socket-apis/19277

Implemented async variants of:
loop.sock_sendmsg - async analogue of socket.sendmsg
loop.sock_recvmsg - async analogue of socket.sock_recvmsg

This is a draft PR open for discussion.

Some open tasks for this draft:

  • Improve documentation
  • More testing
  • Platform independence (developed only on Linux)

@ghost
Copy link

ghost commented Feb 1, 2024

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Feb 1, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@Eclips4 Eclips4 requested a review from gvanrossum February 1, 2024 11:11
@bedevere-app
Copy link

bedevere-app bot commented Feb 1, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

I've only skimmed this, but it looks pretty straightforward (it's just a clone of recvfrom/sendto). I'm still skeptical how much use this would get (the Discourse thread has some folks stating it'd be useful for them, but not a lot). I'm also skeptical that you couldn't easily provide this as a pair of user-level coroutines (yeah, I tried in the Discourse thread and failed, but if this could be a small 3rd party library, it'd take some maintenance off our hands -- asyncio is understaffed as it is).

(@Eclips4 Do you have interest in this yourself or did you just think that this is ready to be promoted from Draft?)

if self._debug and sock.gettimeout() != 0:
raise ValueError("the socket must be non-blocking")
try:
return sock.recvfrom(bufsize)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this use recvmsg? (TBH I don't know anything about recvmsg so I may be missing something. But it looks to me like a simple copy-and-paste bug.)

Copy link
Author

@LukasWoodtli LukasWoodtli Feb 27, 2024

Choose a reason for hiding this comment

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

My bad.
The whole file had a lot of copy-and-paste code already before.
I would love to reduce the code duplications in that module. But probably I will not manage to find time for it.

@h-vetinari
Copy link
Contributor

Targeting the branch of an already released version is probably a nonstarter for a new feature? Should almost certainly be targeting main.

@LukasWoodtli LukasWoodtli force-pushed the gardena/lw/async_recvmsg branch from 8963a9d to d13ffee Compare February 27, 2024 13:21
@LukasWoodtli LukasWoodtli changed the base branch from 3.12 to main February 27, 2024 13:21
@LukasWoodtli LukasWoodtli force-pushed the gardena/lw/async_recvmsg branch from d13ffee to cc88bac Compare February 27, 2024 13:22
@bedevere-app
Copy link

bedevere-app bot commented Feb 27, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@LukasWoodtli LukasWoodtli force-pushed the gardena/lw/async_recvmsg branch from cc88bac to ee826c3 Compare February 27, 2024 14:21
@bedevere-app
Copy link

bedevere-app bot commented Feb 27, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@LukasWoodtli LukasWoodtli force-pushed the gardena/lw/async_recvmsg branch from ee826c3 to fe97e24 Compare February 27, 2024 15:19
@bedevere-app
Copy link

bedevere-app bot commented Feb 27, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@LukasWoodtli LukasWoodtli force-pushed the gardena/lw/async_recvmsg branch from fe97e24 to f92c8cc Compare February 27, 2024 16:05
@bedevere-app
Copy link

bedevere-app bot commented Feb 27, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@1st1
Copy link
Member

1st1 commented Sep 23, 2024

libuv doesn't support these, so it will be a bit of pain to add support for these to uvloop.

All in all, these methods are not the most commonly used, but if one needs to use them, they still can using the add_reader / add_writer low level APIs. For that reason, I don't think these are worthy of adding and growing the API surface.

@gvanrossum
Copy link
Member

libuv doesn't support these, so it will be a bit of pain to add support for these to uvloop.

Hm. It would be the addition of a small amount of pretty standard boilerplate to uvloop, right? Just clone the existing copy for recv and send? I have a slight preference for having asyncio lead and make uvloop follow. Maybe if you provide a small hint, the author of this PR might be able to add this to the next version of uvloop. In the meantime, you can just document as one of the temporary restrictions with uvloop that you can't use recvmsg and sendmsg yet (with an invite to readers to help fix this).

@LukasWoodtli What do you think?

@LukasWoodtli LukasWoodtli force-pushed the gardena/lw/async_recvmsg branch from f92c8cc to 5f7d457 Compare January 22, 2025 19:54
@ghost
Copy link

ghost commented Jan 22, 2025

All commit authors signed the Contributor License Agreement.
CLA signed

@LukasWoodtli LukasWoodtli force-pushed the gardena/lw/async_recvmsg branch from 5f7d457 to 3e421fe Compare January 22, 2025 20:00
@bedevere-app
Copy link

bedevere-app bot commented Jan 22, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@LukasWoodtli
Copy link
Author

I'm a bit undecided about how to proceed with this PR.
On one hand, the standard library seems to be incomplete as it provides some sync API that is not available as an async variant. But on the other hand I see that it doesn't make a lot of sense to maintain some library functions that are only in use by one project.
If there will be more interest in the future, the PR can be reopened again.

@llchan
Copy link

llchan commented Mar 28, 2025

I am interested in seeing this merged. It's a pretty general need that's not bespoke to one downstream project. For example, it's needed to be able to use e.g. IP_PKTINFO in an asyncio context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants