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.

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

[Bug] Cython deadlock on upon being called back with non-callee thread #285

Open
junrushao opened this issue Feb 14, 2021 · 9 comments
Open

Comments

@junrushao
Copy link
Member

No description provided.

@tqchen
Copy link
Contributor

tqchen commented Feb 14, 2021

Was due to the fact that non-python started thread needs to call an explicit state ensure function. Perhaps we should register such functions e.g. runtime.ThreadInit as PackedFunc from cython side

https://docs.python.org/3/c-api/init.html#non-python-created-threads

@junrushao
Copy link
Member Author

@tqchen this is interesting finding! Just curious, is it required by only Cython, or both ctypes and cython?

@tqchen
Copy link
Contributor

tqchen commented Feb 14, 2021

I am not too sure, ctypes's mechanism might already supported this initialization

@junrushao
Copy link
Member Author

junrushao commented Apr 26, 2021

Related issue: apache/tvm#7919

Hmmm...okay, it is not so related...

@tqchen
Copy link
Contributor

tqchen commented Apr 26, 2021

It is related since we can introduce EnvThreadInitialize in the same registry as a followup

@junrushao
Copy link
Member Author

Looks like we have to somehow link to Python to make sure the following snippet could run:

PyGILState_STATE gstate;
gstate = PyGILState_Ensure();

/* Perform Python actions here. */
result = CallSomeFunction();
/* evaluate result or handle exception */

/* Release the thread. No Python API allowed beyond this point. */
PyGILState_Release(gstate);

It might be possible with cython if we register them into two packed functions, but I don't have much idea about ctypes :-(

@junrushao
Copy link
Member Author

@zxybazh Looks like we can actually register these APIs with ctypes: apache/tvm#7919 (comment)

@zxybazh
Copy link
Collaborator

zxybazh commented Oct 16, 2021

Something related: A viable solution for Python concurrency

@junrushao
Copy link
Member Author

It’s a fancy story, which won’t become a real solution in short term

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

No branches or pull requests

3 participants