Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-113812: Allow DatagramTransport.sendto to send empty data#115199

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

ordinary-jamie
Copy link
Contributor

@ordinary-jamieordinary-jamie commentedFeb 9, 2024
edited by github-actionsbot
Loading

UpdateDatagramTransport.sendto method tonot return when data is an empty bytes object. This allows users to send zero-length datagrams (used for example in Time Protocol RFC 868).


📚 Documentation preview 📚:https://cpython-previews--115199.org.readthedocs.build/

@ordinary-jamie
Copy link
ContributorAuthor

@gvanrossum -- This might cause an issue with the flow control of the transport. Since the buffer size is calculated with only the payload (and not the entire datagram).

The write buffer could in theory be flooded with zero-length datagrams, and the high watermark will never be crossed.

self._buffer_size+=len(data)

@gvanrossum
Copy link
Member

@gvanrossum -- This might cause an issue with the flow control of the transport. Since the buffer size is calculated with only the payload (and not the entire datagram).

The write buffer could in theory be flooded with zero-length datagrams, and the high watermark will never be crossed.

Oh, that's a very good point. I think we could add a constant value to the "buffer size" for each packet added -- the only use for the buffer size is to interact with flow control. In fact, after skimming theUDP Wikipedia page, I think we can add 8 for each packet, since that's the protocol's header size.

ordinary-jamie reacted with thumbs up emoji

Copy link
Member

@gvanrossumgvanrossum left a comment

Choose a reason for hiding this comment

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

LGTM, but let's do something about flow control first.

Copy link
Member

@gvanrossumgvanrossum left a comment

Choose a reason for hiding this comment

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

LG -- I'll merge now!

@gvanrossum
Copy link
Member

Sorry, I'd like one more doc change. The code and docs are actually fine, but I feel this deserves a What's New entry (Doc/whatsnew/3.13.rst). A bullet in the existingasyncio section under "Improved Modules" should suffice. I'd like it to mention both the ability to send 0-length packets and the change to buffer size (since it can affect details around flow control). Probably a good idea to mention the latter in the news file too.

ordinary-jamie and synodriver reacted with thumbs up emoji

@ordinary-jamie
Copy link
ContributorAuthor

No worries :) Let me know what you think of this wording I just added!

Copy link
Member

@gvanrossumgvanrossum left a comment

Choose a reason for hiding this comment

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

Thanks! Will merge later tonight.

@ordinary-jamie
Copy link
ContributorAuthor

ordinary-jamie commentedFeb 17, 2024
edited
Loading

Sorry Guido, just pushed a documentation change to for the version change note in the Documentation

Edit: Also pushed a typo fix (repeated "will now") sorry!

@gvanrossumgvanrossum merged commit73e8637 intopython:mainFeb 17, 2024
@gvanrossum
Copy link
Member

Thanks again,@ordinary-jamie!

ordinary-jamie reacted with heart emoji

woodruffw pushed a commit to woodruffw-forks/cpython that referenced this pull requestMar 4, 2024
…ython#115199)Also include the UDP packet header sizes (8 bytes per packet)in the buffer size reported to the flow control subsystem.
diegorusso pushed a commit to diegorusso/cpython that referenced this pull requestApr 17, 2024
…ython#115199)Also include the UDP packet header sizes (8 bytes per packet)in the buffer size reported to the flow control subsystem.
LukasWoodtli pushed a commit to LukasWoodtli/cpython that referenced this pull requestJan 22, 2025
…ython#115199)Also include the UDP packet header sizes (8 bytes per packet)in the buffer size reported to the flow control subsystem.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@gvanrossumgvanrossumgvanrossum approved these changes

@1st11st1Awaiting requested review from 1st11st1 is a code owner

@asvetlovasvetlovAwaiting requested review from asvetlovasvetlov is a code owner

@kumaraditya303kumaraditya303Awaiting requested review from kumaraditya303kumaraditya303 is a code owner

@willingcwillingcAwaiting requested review from willingcwillingc is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@ordinary-jamie@gvanrossum

[8]ページ先頭

©2009-2025 Movatter.jp