GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-113538: Allow client connections to be closed by CendioOssman · Pull Request #114432 · python/cpython

Conversation

@CendioOssman

@CendioOssman CendioOssman commented Jan 22, 2024 •
edited by github-actions Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Give applications the option of more forcefully terminating client connections for asyncio servers. Useful when terminating a service and there is limited time to wait for clients to finish up their work.

  • Issue: Cannot cleanly shut down an asyncio based server #113538

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
CendioOssman added 2 commits January 22, 2024 11:30
@CendioOssman
Don't use internals for wait_closed() tests
9f0111e
@CendioOssman
pythongh-113538: Allow client connections to be closed
c25eabb 
Give applications the option of more forcefully terminating client
connections for asyncio servers. Useful when terminating a service and
there is limited time to wait for clients to finish up their work.
@CendioOssman
CendioOssman requested review from 1st1, asvetlov, gvanrossum, kumaraditya303 and willingc as code owners January 22, 2024 14:56
@bedevere-app bedevere-app Bot added the awaiting review label Jan 22, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Jan 22, 2024
Cannot cleanly shut down an asyncio based server #113538
Closed
@CendioOssman

CendioOssman commented Jan 22, 2024

Copy link
Copy Markdown
Contributor Author

PR includes unit tests, but I also used this more complete example to test and showcase the functionality:

#!/usr/bin/python3

import asyncio
import socket

# Well behaved or spammy protocol?
FLOOD = True

class EchoServerProtocol(asyncio.Protocol):
    def connection_made(self, transport):
        peername = transport.get_extra_info('peername')
        print('Connection from {}'.format(peername))
        self.transport = transport
        self._paused = False
        self._write()

    def pause_writing(self):
        self._paused = True

    def resume_writing(self):
        self._paused = False
        self._write()

    def _write(self):
        if not FLOOD:
            return
        if self._paused:
            return
        self.transport.write(b'a' * 65536)
        asyncio.get_running_loop().call_soon(self._write)

    def data_received(self, data):
        message = data.decode()
        print('Data received: {!r}'.format(message))

def stop():
    loop = asyncio.get_running_loop()
    loop.stop()

client = None

def server_up(fut):
    global server
    server = fut.result()
    print('Server running')
    global client
    client = socket.create_connection(('127.0.0.1', 8888))

loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)

fut = asyncio.ensure_future(loop.create_server(
        lambda: EchoServerProtocol(),
        '127.0.0.1', 8888))
fut.add_done_callback(server_up)

# Simulate termination
loop.call_later(2.5, stop)

try:
    loop.run_forever()
finally:
    print('Closing...')
    server.close()
    try:
        loop.run_until_complete(asyncio.wait_for(server.wait_closed(), 0.1))
    except TimeoutError:
        print('Timeout waiting for clients. Attempting close...')
        server.close_clients()
        try:
            loop.run_until_complete(asyncio.wait_for(server.wait_closed(), 0.1))
        except TimeoutError:
            print('Timeout waiting for client close. Attempting abort...')
            server.abort_clients()
            loop.run_until_complete(asyncio.wait_for(server.wait_closed(), 0.1))
    loop.close()
    if client is not None:
        client.close()
    print('Closed')
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
CendioOssman added 2 commits January 22, 2024 16:02
@CendioOssman
Fix method references in new documentation
b7fa198
@CendioOssman
Add attribution
b3cd9c1
gvanrossum
gvanrossum reviewed Jan 31, 2024
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This looks great, but I have one reservation. The server now keeps the transports alive, through the set of clients. Maybe it would be better if that was a weak set, so that if somehow a transport was freed without going through the usual close routine, the server doesn't hold onto it forever? Thoughts?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@CendioOssman

CendioOssman commented Feb 2, 2024

Copy link
Copy Markdown
Contributor Author

I figured I'd start things simple, so no weak references unless there is a practical issue that requires them.

So, are there any scenarios where this is relevant?

The transport detaches from the server in the same place where it closes the server. That should mean that this is something that must be done, as otherwise the destructor will complain. So I don't think well-written code should have an issue.

Might there be an issue with badly written code, then? The primary case would be if the destructor check fails to trigger. Which I guess is the case with my suggestion. It is difficult to trigger, though. Streams are a bit buggy, so abandoning them doesn't always result in a cleanup (#114914), and transports are referenced by the event loop until you tell them to pause reading.

But difficult isn't impossible, so this case breaks when applying this PR:

#!/usr/bin/python3

import asyncio
import socket

class EchoServerProtocol(asyncio.Protocol):
    def connection_made(self, transport):
        peername = transport.get_extra_info('peername')
        print('Connection from {}'.format(peername))
        self.transport = transport
        self.transport.pause_reading()

    def data_received(self, data):
        message = data.decode()
        print('Data received: {!r}'.format(message))

client = None

def server_up(fut):
    global server
    server = fut.result()
    print('Server running')
    global client
    client = socket.create_connection(('127.0.0.1', 8888))

loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)

fut = asyncio.ensure_future(loop.create_server(
        lambda: EchoServerProtocol(),
        '127.0.0.1', 8888))
fut.add_done_callback(server_up)

# Simulate termination
loop.call_later(2.5, loop.stop)

try:
    loop.run_forever()
    import gc
    gc.collect()
finally:
    server.close()
    loop.close()
    if client is not None:
        client.close()

This case creates a transport, pauses it, and then fails to maintain any other reference to it. Without this PR it gets collected when gc.collect() is called (or earlier). With this PR, it only gets collected at the end.

So a weak reference does indeed seem appropriate.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
CendioOssman added 2 commits February 2, 2024 16:39
@CendioOssman
Only keep a weak client references
c78a927 
We want to be able to detect if the application fails to keep track of
the transports, so we cannot keep them alive by using a hard reference.
@CendioOssman
Inform server of fallback transport cleanup
1ec06da 
The application might be waiting for all transports to close, so we need
to properly inform the server that this transport is done.
@CendioOssman
CendioOssman requested a review from gvanrossum February 2, 2024 16:08
@gvanrossum

gvanrossum commented Feb 19, 2024

Copy link
Copy Markdown
Member

Sorry for the delay -- this just made it to the top of my review queue. I hope to get to this soon!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gvanrossum
gvanrossum reviewed Feb 19, 2024
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Not bad, some thoughts.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/asyncio/base_events.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/whatsnew/3.13.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/asyncio/base_events.py Outdated
self._loop = loop
self._sockets = sockets
self._active_count = 0
# Weak references so abandoned transports can be detected

@gvanrossum gvanrossum Feb 19, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
# Weak references so abandoned transports can be detected
# Weak references so abandoned transports can be ignored

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@CendioOssman CendioOssman Feb 19, 2024

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The wording here was intentional. Weak references is to my knowledge the only way to detect abandoned objects. But it's not this code that does that detection, so I can understand the confusion. How about:

Weak references so we don't break Transport's ability to detect abandoned transports

?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gvanrossum gvanrossum Feb 19, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ah, you're thinking from the POV of the transport, whose __del__ must be called to "detect" (i.e., warn about) that it was abandoned. I was thinking from the POV of the loop in close_clients(), where we want to ignore (not encounter) transports that have been closed already.

I'll make it your choice which wording to use.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/asyncio/selector_events.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Misc/NEWS.d/next/Library/2024-01-22-15-50-58.gh-issue-113538.v2wrwg.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_asyncio/test_server.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_asyncio/test_server.py Outdated

srv.close()
srv.close_clients()
await asyncio.sleep(0.1) # FIXME: flush call_soon()?

@gvanrossum gvanrossum Feb 19, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Yeah, short sleeps are a nuisance in asyncio tests. Usually they can be replaced by a small number of sleep(0) calls though -- usually 1, rarely 2 or 3. sleep(0) is special and guarantees we go through the event loop exactly once.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@CendioOssman CendioOssman Feb 19, 2024

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

A bunch of sleep(0) felt even more arbitrary. :/

What's your suggestion here? Keep it as is? Or something else?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gvanrossum gvanrossum Feb 19, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

A bunch of sleep(0) wastes less time (the asyncio tests are already too slow), and doesn't risk the test becoming flaky due to timing out on slow platforms (a problem we've struggled with). The right number of sleep(0) call takes up no more time than needed, and lets the machinery go through its motions in a deterministic matter. So I recommend sleep(0).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_asyncio/test_server.py Outdated
while s_wr.transport.get_write_buffer_size() == 0:
s_wr.write(b'a' * 65536)
await asyncio.sleep(0)
await asyncio.sleep(0.1) # FIXME: More socket buffer space magically appears?

@gvanrossum gvanrossum Feb 19, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Eh, what's going on with this one? Is it the same issue that can be fixed with a small number of sleep(0) calls, or different?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@CendioOssman CendioOssman Feb 19, 2024

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I don't know, to be honest. Might be a kernel or libc issue where it shuffles buffers around and/or allocates more space.

Without it, I cannot reliably get both the server and client to a state where buffers are full. Which is needed for the test to check the right thing.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gvanrossum gvanrossum Feb 19, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

IMO this PR is not finished until you get to the bottom of that. If you really need then to reach a specific state and there's no deterministic way to get there, consider manipulating internal APIs.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@CendioOssman CendioOssman Feb 27, 2024

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This was a pain, but I think I got it fixed. The core problem is that the kernel dynamically increases the socket send buffer size. And it takes about 20-30 ms to do so.

I think I've worked around that by specifying an explicit buffer size. That should turn off the dynamic resizing, if I remember things correctly.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/library/asyncio-eventloop.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_asyncio/test_server.py Outdated

srv.close()
srv.abort_clients()
await asyncio.sleep(0.1) # FIXME: flush call_soon()?

@gvanrossum gvanrossum Feb 19, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This one looks similar to the first sleep.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
CendioOssman added 3 commits February 19, 2024 13:26
@CendioOssman
Clarify recomended order of methods
2790ddf
@CendioOssman
Fix typos
6a56a80
@CendioOssman
Use discard() instead of conditional remove()
6929888
gvanrossum
gvanrossum reviewed Feb 19, 2024
View reviewed changes
Comment thread Doc/library/asyncio-eventloop.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/asyncio/base_events.py Outdated
self._loop = loop
self._sockets = sockets
self._active_count = 0
# Weak references so abandoned transports can be detected

@gvanrossum gvanrossum Feb 19, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ah, you're thinking from the POV of the transport, whose __del__ must be called to "detect" (i.e., warn about) that it was abandoned. I was thinking from the POV of the loop in close_clients(), where we want to ignore (not encounter) transports that have been closed already.

I'll make it your choice which wording to use.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/asyncio/selector_events.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_asyncio/test_server.py Outdated

srv.close()
srv.close_clients()
await asyncio.sleep(0.1) # FIXME: flush call_soon()?

@gvanrossum gvanrossum Feb 19, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

A bunch of sleep(0) wastes less time (the asyncio tests are already too slow), and doesn't risk the test becoming flaky due to timing out on slow platforms (a problem we've struggled with). The right number of sleep(0) call takes up no more time than needed, and lets the machinery go through its motions in a deterministic matter. So I recommend sleep(0).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_asyncio/test_server.py Outdated
while s_wr.transport.get_write_buffer_size() == 0:
s_wr.write(b'a' * 65536)
await asyncio.sleep(0)
await asyncio.sleep(0.1) # FIXME: More socket buffer space magically appears?

@gvanrossum gvanrossum Feb 19, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

IMO this PR is not finished until you get to the bottom of that. If you really need then to reach a specific state and there's no deterministic way to get there, consider manipulating internal APIs.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/asyncio/base_events.py Outdated
Comment on lines 299 to 300
# Note that 'transport' may already be missing from
# self._clients if it has been garbage collected

@gvanrossum gvanrossum Feb 19, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

IMO you don't need this comment either (it was explaining the condition).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
CendioOssman added 3 commits February 27, 2024 12:52
@CendioOssman
Improve some comments
8316199 
One could be made clearar, and the other is probably superfluous.
@CendioOssman
Do multiple sleeps to flush out callbacks
3e1705b
@CendioOssman
More deterministic abort_clients() test
6c078d6 
Try to get the streams and the kernel in to a more deterministic state
by specifying fixed buffering limits.
@CendioOssman
CendioOssman requested a review from gvanrossum February 27, 2024 14:45
gvanrossum
gvanrossum reviewed Feb 27, 2024
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks for fixing all that. I have one remaining concern only.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_asyncio/test_server.py Outdated
Comment on lines +239 to +246
while c_wr.transport.is_reading():
await asyncio.sleep(0)

# Get the writer in a waiting state by sending data until the
# kernel stops accepting more in to the send buffer
while s_wr.transport.get_write_buffer_size() == 0:
s_wr.write(b'a' * 4096)
await asyncio.sleep(0)

@gvanrossum gvanrossum Feb 27, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Can we put an upper bound on these while loops and fail the test when they loop too many times? (Possibly a for loop with a break is the best idiom to do this.)

I worry that something in the network stack might cause one or the other to loop forever, and I'd rather not waste the CPU time in CI over this.

How many iterations do you expect? Is it deterministic regardless of platform?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
willingc reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@CendioOssman CendioOssman Mar 5, 2024

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It's a bit sensitive to platform behaviour, but I believe I can get something that dynamically adapts

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@CendioOssman
Even more deterministic
1158151 
No possibly infinite loop. Instead ask the system how much buffer space
it has and fill that.
gvanrossum
gvanrossum reviewed Mar 6, 2024
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LG, but can you fix the merge conflict? The tests won't run until that's fixed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@CendioOssman

CendioOssman commented Mar 8, 2024

Copy link
Copy Markdown
Contributor Author

Sure. How do you prefer to handle that? A rebase against current main?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gvanrossum

gvanrossum commented Mar 8, 2024

Copy link
Copy Markdown
Member

No rebase please! Please merge the updated main branch into your branch, fix the conflicts as part of the merge, and then commit and push (not push -f). The extra commits will be squashed when I merge back into main.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@CendioOssman
Merge branch 'main' of https://github.com/python/cpython into server_…
1065dda 
…close
@CendioOssman
CendioOssman requested a review from gvanrossum March 11, 2024 15:15
gvanrossum
gvanrossum approved these changes Mar 11, 2024
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks! LGTM. I'll merge.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Mar 11, 2024
@gvanrossum
gvanrossum merged commit 1d0d49a into python:main Mar 11, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label Mar 11, 2024
@mhsmith

mhsmith commented Mar 11, 2024

Copy link
Copy Markdown
Member

It looks like one of the new tests isn't always passing: #116423 (comment)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gvanrossum added a commit to gvanrossum/cpython that referenced this pull request Mar 11, 2024
@gvanrossum
Revert "pythongh-113538: Add asycio.Server.{close,abort}_clients (pyt…
3c0ecf5 
…hon#114432)"

Reason: The new test doesn't always pass:
python#116423 (comment)

This reverts commit 1d0d49a.
@gvanrossum

gvanrossum commented Mar 11, 2024

Copy link
Copy Markdown
Member

It looks like one of the new tests isn't always passing: #116423 (comment)

Thanks, I've created a new PR on the same issue that will revert it. Sorry for the inconvenience.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gvanrossum added a commit that referenced this pull request Mar 12, 2024
@gvanrossum
gh-113538: Revert "gh-113538: Add asycio.Server.{close,abort}_clients (…
ba13215 
…#114432)" (#116632)

Revert "gh-113538: Add asycio.Server.{close,abort}_clients (#114432)"

Reason: The new test doesn't always pass:
#116423 (comment)

This reverts commit 1d0d49a.
@gvanrossum

gvanrossum commented Mar 12, 2024

Copy link
Copy Markdown
Member

The PR has been reverted.

@CendioOssman could you look into the test failures? I probably should have reviewed the test more carefully, sorry.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@CendioOssman

CendioOssman commented Mar 12, 2024

Copy link
Copy Markdown
Contributor Author

Of course. And once they are fixed, how do you want to proceed? Submit a new PR using the existing branch?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gvanrossum

gvanrossum commented Mar 12, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Of course. And once they are fixed, how do you want to proceed? Submit a new PR using the existing branch?

That sounds fine to me. If you can't get it to work, just rename the branch locally and push that.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@CendioOssman CendioOssman mentioned this pull request Mar 14, 2024
gh-113538: Allow client connections to be closed #116784
Merged
gvanrossum pushed a commit that referenced this pull request Mar 18, 2024
@CendioOssman
gh-113538: Add asycio.Server.{close,abort}_clients (redo) (#116784)
4159644 
These give applications the option of more forcefully terminating client
connections for asyncio servers. Useful when terminating a service and
there is limited time to wait for clients to finish up their work.

This is a do-over with a test fix for gh-114432, which was reverted.
vstinner pushed a commit to vstinner/cpython that referenced this pull request Mar 20, 2024
@CendioOssman @vstinner
pythongh-113538: Add asycio.Server.{close,abort}_clients (redo) (pyth…
cc24c18 
…on#116784)

These give applications the option of more forcefully terminating client
connections for asyncio servers. Useful when terminating a service and
there is limited time to wait for clients to finish up their work.

This is a do-over with a test fix for pythongh-114432, which was reverted.
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
@CendioOssman @adorilson
pythongh-113538: Add asycio.Server.{close,abort}_clients (python#114432)
b6b3853 
These give applications the option of more forcefully terminating client
connections for asyncio servers. Useful when terminating a service and
there is limited time to wait for clients to finish up their work.
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
@gvanrossum @adorilson
pythongh-113538: Revert "pythongh-113538: Add asycio.Server.{close,ab…
e8dd85d 
…ort}_clients (python#114432)" (python#116632)

Revert "pythongh-113538: Add asycio.Server.{close,abort}_clients (python#114432)"

Reason: The new test doesn't always pass:
python#116423 (comment)

This reverts commit 1d0d49a.
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
@CendioOssman @adorilson
pythongh-113538: Add asycio.Server.{close,abort}_clients (redo) (pyth…
ac9d421 
…on#116784)

These give applications the option of more forcefully terminating client
connections for asyncio servers. Useful when terminating a service and
there is limited time to wait for clients to finish up their work.

This is a do-over with a test fix for pythongh-114432, which was reverted.
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
@CendioOssman @diegorusso
pythongh-113538: Add asycio.Server.{close,abort}_clients (python#114432)
2e1b810 
These give applications the option of more forcefully terminating client
connections for asyncio servers. Useful when terminating a service and
there is limited time to wait for clients to finish up their work.
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
@gvanrossum @diegorusso
pythongh-113538: Revert "pythongh-113538: Add asycio.Server.{close,ab…
08737d3 
…ort}_clients (python#114432)" (python#116632)

Revert "pythongh-113538: Add asycio.Server.{close,abort}_clients (python#114432)"

Reason: The new test doesn't always pass:
python#116423 (comment)

This reverts commit 1d0d49a.
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
@CendioOssman @diegorusso
pythongh-113538: Add asycio.Server.{close,abort}_clients (redo) (pyth…
01d051f 
…on#116784)

These give applications the option of more forcefully terminating client
connections for asyncio servers. Useful when terminating a service and
there is limited time to wait for clients to finish up their work.

This is a do-over with a test fix for pythongh-114432, which was reverted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum approved these changes
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner
@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303 kumaraditya303 is a code owner
@willingc willingc Awaiting requested review from willingc willingc is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@CendioOssman @gvanrossum @mhsmith
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.