Skip to content

bpo-43950: ensure source_line is present when specializing the traceback #27313

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Jul 24, 2021
Merged

bpo-43950: ensure source_line is present when specializing the traceback #27313

merged 3 commits into from
Jul 24, 2021

Conversation

isidentical
Copy link
Member

@isidentical isidentical commented Jul 23, 2021 •
edited
Loading

There seems to be different cases where the _PyDisplay_Line() function returns 0 without displaying the actual line but also without having any errors raised.

cpython/Python/traceback.c

Lines 414 to 420 in 4512848

/* use the right encoding to decode the file as unicode */
fd = PyObject_AsFileDescriptor(binary);
if (fd < 0) {
Py_DECREF(io);
Py_DECREF(binary);
return 0;
}

cpython/Python/traceback.c

Lines 436 to 446 in 4512848

if (fob == NULL) {
PyErr_Clear();
res = _PyObject_CallMethodIdNoArgs(binary, &PyId_close);
Py_DECREF(binary);
if (res)
Py_DECREF(res);
else
PyErr_Clear();
return 0;
}

This patch ensures that the line is retrieved and displayed.

https://bugs.python.org/issue43950

@sweeneyde
Copy link
Member

Would it be better to fix _Py_DisplaySourceLine() instead?

@isidentical
Copy link
Member Author

isidentical commented Jul 23, 2021 via email

@pablogsal
Copy link
Member

Can we add a regression test for this? (Like the one on the BPO issue?)

@isidentical isidentical added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 23, 2021
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @isidentical for commit 8db4e0c 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 23, 2021
@isidentical
Copy link
Member Author

It seems like refleaks bots also fail on the main too, so that can be ignored.

@pablogsal pablogsal merged commit c836231 into python:main Jul 24, 2021
@sthagen sthagen mentioned this pull request Jul 24, 2021
Merged
@pablogsal pablogsal mentioned this pull request Apr 13, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pablogsal pablogsal pablogsal approved these changes

@sweeneyde sweeneyde sweeneyde approved these changes

@ammaraskar ammaraskar Awaiting requested review from ammaraskar

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

Successfully merging this pull request may close these issues.
5 participants
@isidentical @sweeneyde @pablogsal @bedevere-bot @the-knights-who-say-ni