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 — bpo-42972: Fix GC assertion error in _winapi by untracking Overlapped earlier by Fidget-Spinner · Pull Request #26429 · python/cpython

Conversation

@Fidget-Spinner

@Fidget-Spinner Fidget-Spinner commented May 28, 2021 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

See #26381 (comment). This untracks a possible 'bad' object earlier so the GC stops having weird race conditions. Fixes a gc assertion error in debug mode in test_httplib.

python_d.exe -m test test_httplib -m test_local_bad_hostname 

...\cpython\Modules\gcmodule.c:446: update_refs: Assertion "gc_get_refs(gc) != 0" failed
Enable tracemalloc to get the memory block allocation traceback

object address  : 000002DADE2A99D0
object refcount : 0
object type     : 000002DADDF72F60
object type name: _winapi.Overlapped
object repr     : <refcnt 0 at 000002DADE2A99D0>

Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed
Python runtime state: initialized

https://bugs.python.org/issue42972

Sorry, something went wrong.

Uh oh!

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

All reactions
@Fidget-Spinner
Fix GC assertion error in _winapi by untracking Overlapped earlier
eb83148
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label May 28, 2021
@bedevere-bot bedevere-bot added the awaiting review label May 28, 2021
@Fidget-Spinner Fidget-Spinner added the skip news label May 28, 2021
@Fidget-Spinner Fidget-Spinner mentioned this pull request May 28, 2021
[3.10] bpo-42972: Fully support GC for _winapi.Overlapped (GH-26381) #26427
Closed
@Fidget-Spinner
Fidget-Spinner requested a review from vstinner May 28, 2021 11:55
@Fidget-Spinner

Fidget-Spinner commented May 28, 2021

Copy link
Copy Markdown
Member Author

@vstinner seems like your gut feeling about UnTrack was right

All reactions

Sorry, something went wrong.

Uh oh!

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

@vstinner

vstinner commented May 28, 2021

Copy link
Copy Markdown
Member

test_httplib doesn't use asyncio, so it's unrelated. The test_httplib crash is: https://bugs.python.org/issue44252

python_d.exe -m test test_httplib -m test_local_bad_hostname 

...\cpython\Modules\gcmodule.c:446: update_refs: Assertion "gc_get_refs(gc) != 0" failed
Enable tracemalloc to get the memory block allocation traceback
All reactions

Sorry, something went wrong.

Uh oh!

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

@vstinner

vstinner commented May 28, 2021 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

If you want to modify overlapped_dealloc(), I suggest to also remove the compatibility code for Windows XP.

    if (self->pending) {
        if (check_CancelIoEx() &&
            Py_CancelIoEx(self->handle, &self->overlapped) &&
            GetOverlappedResult(self->handle, &self->overlapped, &bytes, TRUE))
        {
            /* The operation is no longer pending -- nothing to do. */
        }
        else if (_Py_IsFinalizing())
        {
            /* The operation is still pending -- give a warning.  This
               will probably only happen on Windows XP. */
            PyErr_SetString(PyExc_RuntimeError,
                            "I/O operations still in flight while destroying "
                            "Overlapped object, the process may crash");
            PyErr_WriteUnraisable(NULL);
        }
        else
        {
            /* The operation is still pending, but the process is
               probably about to exit, so we need not worry too much
               about memory leaks.  Leaking self prevents a potential
               crash.  This can happen when a daemon thread is cleaned
               up at exit -- see #19565.  We only expect to get here
               on Windows XP. */
            CloseHandle(self->overlapped.hEvent);
            SetLastError(err);
            return;
        }
    }

check_CancelIoEx() can be removed and CancelIoEx() can be used directly. It's available since Windows Vista and Python 3.11 requires Windows Vista or newer:
https://docs.microsoft.com/en-us/windows/win32/fileio/cancelioex-func

See also bpo-32592 "Drop support of Windows Vista and Windows 7".

Please open a separated PR for that.

I don't think that it's worth it to only move the PyObject_GC_UnTrack() call.

All reactions

Sorry, something went wrong.

Uh oh!

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

@Fidget-Spinner

Fidget-Spinner commented May 28, 2021 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member Author

test_httplib doesn't use asyncio, so it's unrelated. The test_httplib crash is: https://bugs.python.org/issue44252

python_d.exe -m test test_httplib -m test_local_bad_hostname 

...\cpython\Modules\gcmodule.c:446: update_refs: Assertion "gc_get_refs(gc) != 0" failed
Enable tracemalloc to get the memory block allocation traceback

Sorry, I was referring to another crash. These 2 build bots crashed when running in python debug mode due to my previous commit:
https://buildbot.python.org/all/#/builders/146/builds/294/steps/4/logs/stdio
https://buildbot.python.org/all/#/builders/596/builds/323/steps/4/logs/stdio

Both due to assertion errors in the GC. So I wanted to fix those first. The Windows support ones can come another time.

All reactions

Sorry, something went wrong.

Uh oh!

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

@vstinner
vstinner merged commit 490b638 into python:main May 28, 2021
@bedevere-bot bedevere-bot removed the awaiting review label May 28, 2021
@vstinner

vstinner commented May 28, 2021

Copy link
Copy Markdown
Member

Sorry, I was referring to another crash. These 2 build bots crashed when running in python debug mode due to my previous commit:

Ah ok, I merged your PR.

All reactions

Sorry, something went wrong.

Uh oh!

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

@vstinner

vstinner commented May 28, 2021

Copy link
Copy Markdown
Member

Can you please create a 3.10 backport which contains your two commits? Use git cherry-pick -x.

Fidget-Spinner reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

@vstinner

vstinner commented May 28, 2021

Copy link
Copy Markdown
Member

I closed #26431 which only contained the first fix.

All reactions

Sorry, something went wrong.

Uh oh!

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

@Fidget-Spinner
Fidget-Spinner deleted the fix-httplib branch May 28, 2021 16:34
@Fidget-Spinner

Fidget-Spinner commented May 28, 2021

Copy link
Copy Markdown
Member Author

Can you please create a 3.10 backport which contains your two commits? Use git cherry-pick -x.

Yep, #26430 has both fixes. Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

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

@vstinner vstinner mentioned this pull request May 10, 2022
Drop support of Windows Vista and Windows 7 #76773
Closed
@vstinner vstinner mentioned this pull request Jun 9, 2022
[C API] Heap types (PyType_FromSpec) must fully implement the GC protocol #87138
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner Awaiting requested review from vstinner

Uh oh!

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

Assignees

No one assigned

Labels

skip news

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.

4 participants

@Fidget-Spinner @vstinner @the-knights-who-say-ni @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.