Skip to content

Only call PyThread_Ensure from host_task if the main-thread interpreter is initialized #776

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
Feb 11, 2022

Conversation

oleksandr-pavlyk
Copy link
Contributor

@oleksandr-pavlyk oleksandr-pavlyk commented Feb 9, 2022

This PR applies the solution worked out in a toy POC

Host tasks are executed in a separate threads from
the main thread which submitted the kernel and incremented
reference counts.

When kernel submission occurs near the end of the script,
the CPython interpreter may have begun finalization process
by the time the kernel completes the execution.

PyThread_Ensure would crash in that case. So execute the host
task only if Py_IsInitialized().

The only testing of this scenario is to submit the kernel at
the end and not call queue synchronization.

@oleksandr-pavlyk oleksandr-pavlyk changed the title This PR applies the solution worked out in a toy POC Only call PyThread_Ensure from host_task if the main-thread interpreter is initialized Feb 9, 2022
@oleksandr-pavlyk oleksandr-pavlyk force-pushed the fix-crash-in-async-dec-ref branch from 5f235d9 to 76f0f65 Compare February 9, 2022 12:39
This PR applies the solution worked out in a toy POC

Host tasks are executed in a separate thread from
the main thread which submitted the kernel and incremented
reference count on objects we want to keep alive.

When kernel submission occurs near the end of the script,
the CPython interpreter may have begun finalization process
by the time the kernel completes the execution.

`PyThread_Ensure` would crash in that case. So execute the host
task only if `Py_IsInitialized()`.

The only testing of this scenario is to submit the kernel at
the end and not call queue synchronization.
@oleksandr-pavlyk oleksandr-pavlyk force-pushed the fix-crash-in-async-dec-ref branch from 76f0f65 to 27b6d0a Compare February 9, 2022 12:40
@github-actions
Copy link

github-actions bot commented Feb 9, 2022

@coveralls
Copy link
Collaborator

Coverage Status

Coverage remained the same at 81.632% when pulling 27b6d0a on fix-crash-in-async-dec-ref into e5789bf on master.

@oleksandr-pavlyk
Copy link
Contributor Author

@PokhodenkoSA @diptorupd Ping

@oleksandr-pavlyk oleksandr-pavlyk merged commit 902fa01 into master Feb 11, 2022
@github-actions
Copy link

Deleted rendered PR docs from intelpython.github.com/dpctl, latest should be updated shortly. 🤞

@oleksandr-pavlyk oleksandr-pavlyk deleted the fix-crash-in-async-dec-ref branch February 11, 2022 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants