Skip to content

BUG: duplicate message print if warning raises an exception #10257

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 25, 2017

Conversation

mattip
Copy link
Member

@mattip mattip commented Dec 22, 2017

As per the discussion here PyErr_WriteUnraisable(obj) first prints the error context from PyErr_Fetch (which already prints the message) and then prints repr(obj). The old code thus printed the message twice. This PR prints once then, for lack of a better context, shows that the problem originated in an ndarray.

If not too late perhaps this should be backported to 1.14

@eric-wieser
Copy link
Member

If you want to backport this, then it would be easiest if you rebase on 1368cbb

And you missed the BUG: prefix on the commit message, so you'll want to rebase anyway

@@ -475,22 +475,10 @@ array_dealloc(PyArrayObject *self)
"called.";
/* 2017-Nov-10 1.14 */
if (DEPRECATE(msg) < 0) {
/* dealloc must not raise an error, best effort try to write
/* dealloc cannot raise an error, best effort try to write
to stderr and clear the error
Copy link
Member

@eric-wieser eric-wieser Dec 22, 2017

Choose a reason for hiding this comment

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

I think you might need to actually clear the error before calling PyArray_ResolveWritebackIfCopy, in case the array is an object dtype, and PyArray_ResolveWritebackIfCopy causes refcounts to drop to zero which can run arbitrary python code

Copy link
Member

Choose a reason for hiding this comment

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

Although I suppose that's an unrelated fix

Copy link
Member Author

Choose a reason for hiding this comment

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

In the implementation of PyErr_WriteUnraisable it calls PyErr_Fetch which clears the error state

@mattip
Copy link
Member Author

mattip commented Dec 22, 2017

hopefully rebased correctly, off the first fix to PR #9639

@charris charris added the 09 - Backport-Candidate PRs tagged should be backported label Dec 25, 2017
@charris charris merged commit 59084fa into numpy:master Dec 25, 2017
@charris
Copy link
Member

charris commented Dec 25, 2017

Thanks @mattip.

@mattip mattip deleted the fix-PR9639-2 branch December 26, 2017 18:32
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Jan 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants