KEMBAR78
Avoid throwing exception in ClosingTHPObjectPtr by ezyang · Pull Request #109758 · pytorch/pytorch · GitHub
Skip to content

Conversation

@ezyang
Copy link
Contributor

@ezyang ezyang commented Sep 21, 2023

Stack from ghstack (oldest at bottom):

Previously, if ClosingTHPObjectPtr was destructed because we
were unwinding the stack from an exception, we would attempt to call
close() which just isn't going to work. Two fixes:

  1. Detect if we're unwinding due to a Python error, and don't try
    to do more Python stuff if so.

  2. If close() fails somehow, write an unraisable exception, don't
    try to throw because that will terminate if you're in an
    exception.

Signed-off-by: Edward Z. Yang ezyang@meta.com

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @chenyang78 @aakhundov @kadeng

Previously, if ClosingTHPObjectPtr was destructed because we
were unwinding the stack from an exception, we would attempt to call
close() which just isn't going to work.  Two fixes:

1. Detect if we're unwinding due to a Python error, and don't try
   to do more Python stuff if so.

2. If close() fails somehow, write an unraisable exception, don't
   try to throw because that will terminate if you're in an
   exception.

Signed-off-by: Edward Z. Yang <ezyang@meta.com>

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Sep 21, 2023

🔗 Helpful Links

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

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

✅ You can merge normally! (1 Unrelated Failure)

As of commit 93e5f54 with merge base 9c2715b (image):

FLAKY - The following job failed but was likely due to flakiness present on trunk:

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

ezyang added a commit that referenced this pull request Sep 21, 2023
Previously, if ClosingTHPObjectPtr was destructed because we
were unwinding the stack from an exception, we would attempt to call
close() which just isn't going to work.  Two fixes:

1. Detect if we're unwinding due to a Python error, and don't try
   to do more Python stuff if so.

2. If close() fails somehow, write an unraisable exception, don't
   try to throw because that will terminate if you're in an
   exception.

Signed-off-by: Edward Z. Yang <ezyangmeta.com>

ghstack-source-id: b445aab
Pull Request resolved: #109758
@github-actions github-actions bot requested a review from albanD September 21, 2023 00:30
@ezyang ezyang requested a review from jansel September 21, 2023 00:55
@ezyang
Copy link
Contributor Author

ezyang commented Sep 21, 2023

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 21, 2023
@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team Raised by workflow job

@ezyang
Copy link
Contributor Author

ezyang commented Sep 21, 2023

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@facebook-github-bot facebook-github-bot deleted the gh/ezyang/2362/head branch September 25, 2023 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants