Skip to content
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

Use the Python frame safely in _pythonCallstack (#88993) #89927

Closed

Conversation

weiwangmeta
Copy link
Contributor

Currently, the result of PyEval_GetFrame() is piped straight to Py_INCREF. However, PyEval_GetFrame may return null, which seems to be the case sometimes, when calling _pythonCallstack from another thread. This is handled in the subsequent while (nullptr != frame) block, but Py_INCREF, called before it, doesn't handle this case, so the program segfaults. The safe form of Py_INCREF is Py_XINCREF, so use that instead (docs).
Pull Request resolved: #88993
Approved by: https://github.com/albanD

Fixes #ISSUE_NUMBER

Currently, the result of `PyEval_GetFrame()` is piped straight to `Py_INCREF`. However, `PyEval_GetFrame` [may return null](https://docs.python.org/3/c-api/reflection.html#c.PyEval_GetFrame), which seems to be the case sometimes, when calling `_pythonCallstack` from another thread. This is handled in the subsequent `while (nullptr != frame)` block, but `Py_INCREF`, called before it, [doesn't handle this case](https://docs.python.org/3/c-api/refcounting.html#c.Py_INCREF), so the program segfaults. The safe form of `Py_INCREF` is `Py_XINCREF`, so use that instead ([docs](https://docs.python.org/3/c-api/refcounting.html#c.Py_XINCREF)).
Pull Request resolved: pytorch#88993
Approved by: https://github.com/albanD
@pytorch-bot
Copy link

pytorch-bot bot commented Nov 30, 2022

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/89927

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit afd23b9:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@pytorch-bot pytorch-bot bot added the release notes: jit release notes category label Nov 30, 2022
@weiwangmeta
Copy link
Contributor Author

Closing in favor of #89997

But why is it only triggering 12 to 13 checks?

@weiwangmeta weiwangmeta closed this Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: jit release notes category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants