Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

added missing PyThreadState_Swap to Runtime and its Delegates#1673

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

Conversation

eirannejad
Copy link
Contributor

What does this implement/fix? Explain your changes.

PyThreadState_Swap was missing inRuntime andRuntime.Delegates code

Does this close any currently open issues?

No

Any other comments?

N/A

Checklist

Check all those that are applicable and complete.

  • Make sure to include one or more tests for your change
    ⚠️ Skipping tests since there are not any tests for specific runtime p/invokes. This method is not used anywhere in original pythonnet code base as of creating this PR
  • If an enhancement PR, please create docs and at best an example
  • Add yourself toAUTHORS
  • Updated theCHANGELOG

@lostmsu
Copy link
Member

lostmsu commentedJan 15, 2022
edited
Loading

This looks good but without usages there's not much point merging it. We only have P/Invoke signatures for functions that are used byPython.Runtime.

@eirannejad
Copy link
ContributorAuthor

@lostmsu I'm using this in a fork that I have working with sub-interpreters. Whichever you decide. I thought it's better to PR to master

@lostmsu
Copy link
Member

Are you planning to upstream subinterpreters? Or is it not yet ready?

@lostmsu
Copy link
Member

We are about to release 3.0 with breaking changes. If subinterpreters require more breaking changes, it might make sense to try getting them in, otherwise you might need to wait until 4.0

eirannejad reacted with heart emoji

@eirannejad
Copy link
ContributorAuthor

eirannejad commentedJan 15, 2022
edited
Loading

@lostmsu I definitely am. To be honest I'm a little in the dark as to what is the overall scope and design goal for pythonnet. I'd appreciate your help and info on this. It'd help me push more generic code upstream.

Q: IsPythonEngine intended to be a facade over the runtime and wrap more complex functionality, simplifying runtime usage? or is it intended to be as close as possible to python runtime itself?Runtime.cs currently wrapsPy_NewInterpreter andPy_EndInterpreter without them being used in pythonnet code AFAIK. I'm not sure if I should just wrap these inPythonEngine or provide acontext manager like this to simplify usage?

  • The linked code is custom to my specific use case but I'd love to make it more generic and push upstream
  • It's more of an overall design api goal question really.

@lostmsu
Copy link
Member

Actually,this passage does not look promising:

Furthermore, extensions (such asctypes) using these APIs to allow calling of Python code from non-Python created threads will probably be broken when using sub-interpreters.

I wonder what is your use case? IsPyScope not sufficient?

@eirannejad
Copy link
ContributorAuthor

eirannejad commentedJan 15, 2022
edited
Loading

@lostmsu I'm embedding pythonnet in a dotnet application and creating a cpython IDE over it. I need to run a mini language server on the python engine as well that doesn't have anything to do with the actual user scripts being executed by the engine. Effectively this creates two completely independent usages for pythonnet under the same process. I need to create a sub-interpreter for these reasons:

  • The language server imports lots of support libraries for linting and autocompletion. If this is just anotherPyScope in the main interpreter it would sprinklesys.modules with those imported modules
  • I need to assign independentstdout andstderr streams to this language server and therefore need a separate interpreter

Please let me know if there is a better way to do this

P.S. I don't seePyScope in pythonnet3 anymore. Seems like it isPyModule now right?

@filmor
Copy link
Member

Regarding PythonEngine:

We will have to break this one eventually anyhow if we ever want subinterpreters, so if you want to make a proposal, go ahead. From my perspective, it only exists for compatibility reasons, all newer functionalities were added to the relatively ergonomicPy class. I'd suggest leavingPythonEngine as is (marking it as obsolete) and creating a new classPyInterpreter orPySubInterpreter to carry theEval andImport etc. functions on an instance instead ofstatic and just pointingPy.Eval etc. to aPy.MainInterpreter.

Regarding PyScope:

This is indeedPyModule now, there was a lot of overlap:#1569

eirannejad reacted with thumbs up emoji

@lostmsu
Copy link
Member

My understanding is that the issue requires more involved changes.

For modules using single-phase initialization, e.g.PyModule_Create() (this is what Python.NET does AFAIR), the first time a particular extension is imported, it is initialized normally, and a (shallow) copy of its module’s dictionary is squirreled away. When the same extension is imported by another (sub-)interpreter, a new module is initialized and filled with the contents of this copy; the extension’s init function is not called. Objects in the module’s dictionary thus end up shared across (sub-)interpreters, which might cause unwanted behavior (see Bugs and caveats below).

Now if you think about it, the modules that Python.NET creates to reflect .NET namespaces if simply copied to the new subinterpreter won't get notified of newly loaded assemblies.

The following passage is also unclear to the exact effect:

Also note that combining this functionality with PyGILState_* APIs is delicate, because these APIs assume a bijection between Python thread states and OS-level threads, an assumption broken by the presence of sub-interpreters. It is highly recommended that you don’t switch sub-interpreters between a pair of matching PyGILState_Ensure() and PyGILState_Release() calls. Furthermore, extensions (such as ctypes) using these APIs to allow calling of Python code from non-Python created threads will probably be broken when using sub-interpreters.

We can probably defer that issue to users, but the problem is that because most usages of Python.NET are higher-level, we will get a lot of people asking why it does not work like they expect it should.

BTW,@eirannejad, according to the first quote above in the example you mentioned, Python.NET will only be initialized once.

As for running a language server, why not useLanguage Server Protocol used in VS Code for all languages. Then you should be able to just reuse existing Python language server that Microsoft developed for VS Code. This does not use subinterpreters, and instead just runs language server library in a separate process.

AFAIK, Python language server in VS Code does not autocomplete .NET types, and this is something we might want to address instead.

@lostmsu
Copy link
Member

As discussed, we are not taking it in the current form. I hope the LSP is good alternative for you@eirannejad

eirannejad reacted with thumbs up emoji

@lostmsulostmsu closed thisMay 7, 2022
@eirannejadeirannejad deleted the dev/fixmissingswap branchJuly 2, 2022 23:25
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@eirannejad@lostmsu@filmor

[8]ページ先頭

©2009-2025 Movatter.jp