ninia / jep

Embed Python in Java
Other
1.31k stars 149 forks source link

Add optional to allow execution script in different thread #499

Closed ddovnar closed 1 year ago

ddovnar commented 1 year ago

Hi! I'm ran into a problem with situation, when I trying to call multiple short scripts in the created singleton Interpreter. To resolve this issue, I added optional flag to disable checking of currentThread in the isValidThread method.

bsteffensmeier commented 1 year ago
  1. This change will cause the same PyThreadState to be active on multiple threads. The cpython documentation indicates that each thread should create it's own PyThreadState so I don't think this is a supported way of using the API. My biggest concern is that cpython maintains a pointer to the current thread state and this is managed with the same API as the GIL. I am not confident the GIL will work as intended when reusing PyThreadState like this.
  2. Have you done testing on this change using third party python extensions that use the PyGILState_* C-API? The reason for most of the threading constraints related to jep are because we have found cases where cpython will deadlock when PyGIlState_Ensure is called if there are multiple interpreters. This change seems to bypass all the protections we put in place to prevent deadlock so I am pretty sure there will be ways to deadlock in Jep with these changes. We will not accept this change if it re-introduces the deadlock problems we previously fixed with the threading constraints.