◐ Shell
reader mode source ↗
Skip to content

Add GetPythonThreadID and Interrupt methods in PythonEngine#1337

Merged
lostmsu merged 4 commits into
pythonnet:masterfrom
gpetrou:Interrupt
Jan 21, 2021
Merged

Add GetPythonThreadID and Interrupt methods in PythonEngine#1337
lostmsu merged 4 commits into
pythonnet:masterfrom
gpetrou:Interrupt

Conversation

@gpetrou

@gpetrou gpetrou commented Dec 26, 2020

Copy link
Copy Markdown
Contributor

What does this implement/fix? Explain your changes.

Fixes #766 by adding an Interrupt method in PythonEngine class.

Checklist

Check all those that are applicable and complete.

  • Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example
  • Add yourself to AUTHORS
  • Updated the CHANGELOG

@gpetrou gpetrou force-pushed the Interrupt branch 9 times, most recently from 9b35013 to d075db3 Compare December 26, 2020 16:34
@filmor

filmor commented Dec 26, 2020

Copy link
Copy Markdown
Member

Copying my comment from #1333:

  1. This requires a test (done)
  2. Strictly, the thread-id attribute is unsigned only for Python >= 3.7 and we still support 3.6
  3. Also, since the original type is long, the "correct" type is (U)IntPtr on everything but Windows (Not quite sure how far we have to go there compatibility-wise, the current implementation could be fine for the usual thread IDs)
  4. Maybe we should just expose the API function directly (i.e. RaiseInPythonThread on Exception objects or so) and make Interrupt use that

Additional comments on the current state: The helper API to get the current thread id should be in the library and should use threading.get_native_id() on Python >=3.8.

@filmor

filmor commented Dec 26, 2020

Copy link
Copy Markdown
Member

It would also be good if you'd just use normal commits now without force-pushing a single commit over and over, otherwise it becomes very difficult to follow your changes. We can (and will) squash this in the end.

@gpetrou

gpetrou commented Dec 27, 2020

Copy link
Copy Markdown
Contributor Author

Please note that I am very unfamiliar with the codebase, so I will need more explanations from you. With regards to:
2. I added a version check. Is this what you mean?
3. I added different calls per OS and version. Is this what you mean?
4. I don't understand what that means. Can you elaborate? Is this still needed?

@gpetrou gpetrou force-pushed the Interrupt branch 3 times, most recently from 07b9655 to c3f7c78 Compare December 27, 2020 07:17
@filmor

filmor commented Dec 27, 2020

Copy link
Copy Markdown
Member

This goes in the right direction :)

Regarding 2.: Yes, that's what I meant.
Regarding 3.: This looks better, but the Windows type is uint and int. .NET's int is always 32bit, and on Windows long is also 32bit on both x86 and x64.

@gpetrou gpetrou force-pushed the Interrupt branch 8 times, most recently from fb52aa3 to 90a08b6 Compare December 31, 2020 09:08
@gpetrou

gpetrou commented Dec 31, 2020

Copy link
Copy Markdown
Contributor Author

@filmor can this be merged now?

23 hidden items Load more…
@gpetrou gpetrou force-pushed the Interrupt branch 2 times, most recently from ce3d6b4 to 9b18f26 Compare January 8, 2021 07:00
@gpetrou gpetrou changed the title Add Interrupt method in PythonEngine Jan 21, 2021
@gpetrou gpetrou force-pushed the Interrupt branch 2 times, most recently from 6bcda67 to c455ee4 Compare January 21, 2021 09:00
@lostmsu lostmsu merged commit 5fd77b1 into pythonnet:master Jan 21, 2021
lostmsu added a commit to losttech/pythonnet that referenced this pull request Feb 21, 2021
lostmsu added a commit to losttech/pythonnet that referenced this pull request Feb 21, 2021
lostmsu added a commit to losttech/pythonnet that referenced this pull request Feb 21, 2021
lostmsu added a commit to losttech/pythonnet that referenced this pull request Feb 21, 2021
lostmsu added a commit that referenced this pull request Feb 21, 2021
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.

Interrupting running Python code from another thread

3 participants