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.

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

Correctly set the exit code when an exception is raised in the atexit callback _prepare_to_exit. #6383

Merged
merged 11 commits into from
Feb 9, 2024
Merged

Correctly set the exit code when an exception is raised in the atexit callback _prepare_to_exit. #6383

Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ebb4848
re-apply https://github.com/pytorch/xla/pull/6247
vanbasten23 Jan 25, 2024
01735f3
make the test shorter
vanbasten23 Jan 26, 2024
fdc8e9b
set the exit code to 1 and let's check the ci status
vanbasten23 Feb 5, 2024
a2c4167
make sure we flush printing the exception
vanbasten23 Feb 5, 2024
cba8e5d
revert the repro change
vanbasten23 Feb 6, 2024
cf3742d
fix typo
vanbasten23 Feb 6, 2024
7471921
fix comment
vanbasten23 Feb 8, 2024
6c9277f
fix linter
vanbasten23 Feb 8, 2024
e89769d
add some comment
vanbasten23 Feb 8, 2024
bf6686d
add some comment
vanbasten23 Feb 8, 2024
d22eaf0
fix typo
vanbasten23 Feb 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions torch_xla/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,17 @@ def _setup_tpu_vm_library_path() -> bool:


def _prepare_to_exit():
_XLAC._prepare_to_exit()
if int(os.environ.get('PT_XLA_DEBUG', '0')):
_summarize_fn_tracker()
try:
_XLAC._prepare_to_exit()
if int(os.environ.get('PT_XLA_DEBUG', '0')):
_summarize_fn_tracker()
except Exception as e:
logging.error(
"Caught an exception when exiting the process. Exception: ", exc_info=e)
# Due to https://bugs.python.org/issue27035, simply raising an exception in the atexit callback does not set the exit code correctly. That is why we need to set the exit code explicitly.
# Using `exit(1)` does not set a correct exit code because it is useful for the interactive interpreter shell and should not be used in programs and it works by raising an exception. (https://docs.python.org/3/library/constants.html#exit)
# sys.exit(1) does not set a correct exit code because it also raises an exception.
os._exit(1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

sorry, one more micro-nit: make this exit(1)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried and it didn't set the exit code correctly. Per https://docs.python.org/3/library/constants.html#exit, calling exit(1) raises a SystemExit exception. But due to https://bugs.python.org/issue27035, raising an exception won't set the exit code correctly.

Also, per https://docs.python.org/3/library/constants.html#exit, exit or quit "are useful for the interactive interpreter shell and should not be used in programs.".

I'll add a comment.



def _init_xla_lazy_backend():
Expand Down
Loading