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-94609: Make test_ssl.ThreadedEchoServer exceptions appear in a main thread by arhadthedev · Pull Request #92475 · python/cpython

Conversation

@arhadthedev

@arhadthedev arhadthedev commented May 8, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Feature or enhancement

This PR:

  • adds test.test_ssl.Server that is like test.test_ssl.ThreadedEchoServer but (1) implemented as a context manager and (2) replaces plain printing of server-side tracebacks with bubbling the exception up between threads through a future. The (2) is implemented as follows:
    • the server force-closes a client connection when exceptions occur so a client thread instantly gets ConnectionAbortedError / ConnectionResetError hence knowing that the future is going to have a server exception inside
    • the client-side context manager gets the connection failure and rethrows the prepared server exception from the future
  • converts some test cases into using the new approach; others will be converted later in separate PRs
  • enforses a timeout for all socket operations using setUp/tearDown
  • teaches tests to push empty packets because TLS 1.3 delays the handshake until some data are sent

Pitch

  • expected exceptions (caused by *_fail test cases) do not flood a console with unnecessary stack traces (and a heart of a watcher with fear)
  • instead, they can be captured by assertRaises in a main thread; unexpected exceptions will migrate into a Tests Result report footnote failing the run so no need to scroll the log hundreds lines up
  • a server-side exception won't hang a main thread until a socket reading timeout happens
  • replacement of test_ssl and test_asyncio threaded servers with a unified implementation (they cannot be ported to asyncio because a client part is blocking and requires thread separation anyway)

TODO

Plans for future PRs:

  • port all test cases to the new class
  • remove the original ThreadedEchoServer
  • port other test suits that use multithreaded servers (including the ones that currently use asyncore like test_ftp and test_logging)
  • Issue: test_ssl is too verbose #94609

Sorry, something went wrong.

Uh oh!

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

All reactions
@bedevere-bot bedevere-bot added the awaiting review label May 8, 2022
arhadthedev added 4 commits May 19, 2022 12:28
@arhadthedev
Add a threaded server factory
ea5230f
Instead of threads and flags, employ high-level concurrency primitives
like tasks and futures.

A thread pool allows to run tests in parallel so it is future-proof.
@arhadthedev
Add an alternative implementation of ThreadedEchoServer
c6e68e9
Start delegating all exceptions to a main, tester thread.

- thread creation for each new client has no use in single-shot
  disposable servers that need to be created fresh for every test case
  anyway

- stop-flag has no use either; there is no interactive session as well
  as waiting for other users. To test server-side connection abruption,
  instruct a server instance in advance

- wrap_conn either modified self.sslconn and returned True, or swallowed
  exceptions and returned False. Now it either return sslconn directly
  or lets an exception to bubble up through a future into a main thread.
  The main thread, in its turn, either expects it with assertRaises() or
  lets it bubble further so the test case is reported as failed

- transparent bubbling up means no explicit collection of error
  messages in conn_errors. Let unittest package do this job for us

- universal read-write-close operations are changed from inner ifs to
  assignment of specific lambda functions

- constant repetition of `if support.verbose and chatty:` //
  `sys.stdout.write(' server: ...\n')` is moved into log()

- chatty and connectionchatty flags are merged to form a single story of
  communication between a client and a server

- the first usage of ThreadedEchoServer is on line 1286 while the server
  itself was declared starting with line 2557; the new implementation
  is put before the first usage for sequential narration
@arhadthedev
Port two simple cases as an example
47ac053
In ThreadedEchoServer chatty=True by default so omit it.
@arhadthedev
Port multiserver test_check_hostname
05fd819
@arhadthedev
arhadthedev force-pushed the threadedserver branch 2 times, most recently from 1ef0c2c to 610e8e1 Compare May 19, 2022 09:44
arhadthedev added 4 commits May 19, 2022 12:47
@arhadthedev
Port a case where client connects twice
149d18c
@arhadthedev
Port a case where client connects four times
e97b07c
@arhadthedev
Fix a race between client connection and server launch
4fa30f1
Force clients to wait on a server socket while the server worker thread
starts up, instead of erroring on unavailable host. For this, open
the socket in a main thread.

Server socket timeout has no use because the only operation it limits
is listen(). To have it permawaiting we need a main thread interrupted
without terminating the whole program; that's possible under a debugger
only.
@arhadthedev
Add a temporary workaround for zero-byte SSL transfers
d5c4762
@arhadthedev
arhadthedev force-pushed the threadedserver branch from 610e8e1 to d5c4762 Compare May 19, 2022 15:39
arhadthedev added 8 commits May 21, 2022 19:06
@arhadthedev
Prevent hanging in case of client overexpectation
289e563
@arhadthedev
Fix more sporadic errors
9aa2bef
@arhadthedev
Merge branch 'main' into threadedserver
bb750f3
@arhadthedev
Automatic thread management not only needs no manual leak detector, i…
d39fba9
…t breaks it with a long-live thread pool
@arhadthedev
Use a named constant for a timeout
a2f0b07
@arhadthedev
Restore an unstable test
bbce667
@arhadthedev
Restore the second unstable test
d10cc72
@arhadthedev
Remove accidentally included comment fragment
18b3c15
@arhadthedev arhadthedev changed the title Make test_ssl.ThreadedEchoServer exceptions appear in a main thread gh-94609: Make test_ssl.ThreadedEchoServer exceptions appear in a main thread Jul 6, 2022
arhadthedev added 4 commits July 6, 2022 19:35
@arhadthedev
Add a NEWS entry
1c8a6f1
@arhadthedev
Fix a potential method name conflict between base and derived classes
bbb7c6f
@arhadthedev
Remove a redundant parameter
beb1650
@arhadthedev
Revert "Restore the second unstable test"
4b4b4e5
This reverts commit d10cc72.
@arhadthedev

arhadthedev commented Jul 6, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member Author

A thread pool triggers ENV CHANGED. Any ideas how to remedy this without creating&destroying a loop for each individual test?

== Tests result: ENV CHANGED ==
425 tests OK.
10 slowest tests:
- test_tools: 5 min 45 sec
- test_concurrent_futures: 2 min 54 sec
- test_multiprocessing_spawn: 2 min 40 sec
- test_gdb: 2 min 7 sec
- test_multiprocessing_forkserver: 1 min 28 sec
- test_asyncio: 1 min 27 sec
- test_venv: 1 min 17 sec
- test_multiprocessing_fork: 1 min 17 sec
- test_signal: 1 min 5 sec
- test_regrtest: 50.5 sec
1 test altered the execution environment:
    test_ssl
10 tests skipped:
    test_devpoll test_ioctl test_kqueue test_launcher test_msilib
    test_startfile test_winconsoleio test_winreg test_winsound
    test_zipfile64
Total duration: 11 min 1 sec
Tests result: ENV CHANGED
make: *** [Makefile:1863: buildbottest] Error 3
Error: Process completed with exit code 2.
All reactions

Sorry, something went wrong.

Uh oh!

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

arhadthedev added 2 commits July 6, 2022 21:55
@arhadthedev
Forcefully release a thread after a module is over
0f3663b
@arhadthedev
Account for a fact that assignments cannot be inside lambdas
a999eab
@arhadthedev
arhadthedev marked this pull request as ready for review July 6, 2022 19:23
@arhadthedev

arhadthedev commented Jul 6, 2022

Copy link
Copy Markdown
Member Author

Any ideas how to remedy this without creating&destroying a loop for each individual test?

Not relevant anymore. The creationa and destruction happens for some modules only.

Whatever, it's ready for a review.

All reactions

Sorry, something went wrong.

Uh oh!

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

arhadthedev added 2 commits July 10, 2022 09:07
@arhadthedev
Restore explicit server timeout; getdefaulttimeout does not propagate…
cc1b400
… between threads
@arhadthedev
Try to fix the ENV CHANGED error probably caused by cleanup misorder
fd0468e
@arhadthedev arhadthedev mentioned this pull request Jul 10, 2022
NotImplementedError should be thrown on feeding inappripriate objects into the with statement #94724
Closed
arhadthedev added 10 commits July 10, 2022 09:52
@arhadthedev
Add some debug scaffolding to track down a leaked thread
de27807
@arhadthedev
Avoid thread leaks by shutting down the pool
b410384
@arhadthedev
Revert "Avoid thread leaks by shutting down the pool"
2a44b2a
This reverts commit b410384.
@arhadthedev
Remove threading control (a thread pool interferes with it)
1f6378a
@arhadthedev
Revert "Add some debug scaffolding to track down a leaked thread"
4f7b0e9
@arhadthedev
On MacOS server that lost a client still raises an exception
f0e7c8d
@arhadthedev
Clarify terms in NEWS
c67bb1a
@arhadthedev
Streamline comments
cdb9137
@arhadthedev
Set a default socket timeout globally
fbacdc8
@arhadthedev
Merge branch 'main' into threadedserver
d6940b9
@arhadthedev
arhadthedev marked this pull request as ready for review July 10, 2022 16:14
@arhadthedev

arhadthedev commented Jul 10, 2022

Copy link
Copy Markdown
Member Author

Everything is green and ready for a review (after the release blocker crunch is over, of course).

All reactions

Sorry, something went wrong.

Uh oh!

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

@arhadthedev
Use sockets directly, without contextlib.closing
e526af4
@arhadthedev arhadthedev mentioned this pull request Jul 12, 2022
tests still rely on the removed stdlib modules asyncore/asynchat #93243
Closed
2 of 12 tasks
@arhadthedev
Merge branch 'main' into threadedserver
78d725e
@arhadthedev

arhadthedev commented Jul 29, 2022

Copy link
Copy Markdown
Member Author

@tiran Would you mind to look at this PR? It adds a unified separate-threaded test.support.threading_helper.Server that I'm planning to use as a foundation to replace asynchat-based servers where asyncio is not applicable.

All reactions

Sorry, something went wrong.

Uh oh!

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

arhadthedev added 2 commits October 30, 2022 18:57
@arhadthedev
Put the news entry into a present tense
b8174ae
@arhadthedev
Merge branch 'main' into threadedserver
a551384
arhadthedev
arhadthedev commented Oct 30, 2022
View reviewed changes
Comment thread Lib/test/support/threading_helper.py
Comment on lines +17 to +25
def _release():
global _thread_pool
_thread_pool = None


def init():
global _thread_pool
_thread_pool = ThreadPoolExecutor()
unittest.addModuleCleanup(_release)

@arhadthedev arhadthedev Oct 30, 2022

Copy link
Copy Markdown
Member 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've added init() to avoid a modified environment test failure due to a leaked object. If there's a better solution, I'll use that instead.

Sorry, something went wrong.

Uh oh!

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

All reactions
@arhadthedev
Merge branch 'main' into threadedserver
a3693f3
@bedevere-bot bedevere-bot mentioned this pull request Nov 27, 2022
test_ssl is too verbose #94609
Open
@arhadthedev
Merge branch 'main' into threadedserver
e4d35a5
@github-actions

github-actions Bot commented Apr 11, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

All reactions

Sorry, something went wrong.

Uh oh!

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

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 11, 2026
@arhadthedev

arhadthedev commented Oct 24, 2026

Copy link
Copy Markdown
Member Author

Closing; I suspect the test module changed too much last four years so it would be easier to redo it from scratch.

All reactions

Sorry, something went wrong.

Uh oh!

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

@arhadthedev arhadthedev closed this Oct 24, 2026
@arhadthedev
arhadthedev deleted the threadedserver branch October 24, 2026 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

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

Assignees

No one assigned

Labels

awaiting review stale Stale PR or inactive for long period of time.

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.

2 participants

@arhadthedev @bedevere-bot
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.