Skip to content

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

Merged
oleksandr-pavlyk merged 1 commit intomasterfrom
fix-crash-in-async-dec-ref
Feb 11, 2022
Merged

Only call PyThread_Ensure from host_task if the main-thread interpreter is initialized#776
oleksandr-pavlyk merged 1 commit intomasterfrom
fix-crash-in-async-dec-ref

Conversation

@oleksandr-pavlyk
Copy link
Copy Markdown
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
Copy Markdown

github-actions Bot commented Feb 9, 2022

@coveralls
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

@PokhodenkoSA @diptorupd Ping

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

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