[Lldb-commits] [lldb] r355406 - Revert "Fix embedded Python initialization according to changes in version 3.7"

[Tatyana Krasnukha via lldb-commits]

Davide, Stella, thank you for investigating the issue!
I'll re-submit changes when the build will be recovered from compilation errors.

-----Original Message-----
From: Stella Stamenova <stilis at microsoft.com> 
Sent: Wednesday, March 6, 2019 2:06 AM
To: Davide Italiano <dccitaliano at gmail.com>
Cc: Tatyana Krasnukha <tatyana.krasnukha at synopsys.com>; lldb-commits <lldb-commits at lists.llvm.org>
Subject: RE: [Lldb-commits] [lldb] r355406 - Revert "Fix embedded Python initialization according to changes in version 3.7"

It should be safe to checkin the change now. It looks like at some point during the last couple of days the timing of some of the threads tests on Windows changed so now instead of behaving and passing or failing nicely, they are getting struck and never completing. This does not happen on every run and the python change just happened to coincide with some of the failures.

I've disabled the two tests that seemed to be the cause of the issue, so that the bot can go back to green.

Thanks
-Stella

-----Original Message-----
From: Stella Stamenova
Sent: Tuesday, March 5, 2019 10:26 AM
To: 'Davide Italiano' <dccitaliano at gmail.com>
Cc: Tatyana Krasnukha <tatyana at synopsys.com>; lldb-commits <lldb-commits at lists.llvm.org>
Subject: RE: [Lldb-commits] [lldb] r355406 - Revert "Fix embedded Python initialization according to changes in version 3.7"

I ran the tests without that line on a machine that is equivalent to the Buildbot and they ran correctly, but I just noticed that while the Buildbot has had several runs that did not time out since the revert, there was also one that did time out.

I am going to let the current build on the Buildbot complete (since it should be green as a fix was committed for the other failure) and I'm going to restart it afterwards.

In the mean time, I am running the tests on the machine that is the same with the full python 3.7 change to confirm whether it hangs there too or if it was somehow bot specific.

Thanks,
-Stella

-----Original Message-----
From: Davide Italiano <dccitaliano at gmail.com>
Sent: Tuesday, March 5, 2019 8:43 AM
To: Stella Stamenova <stilis at microsoft.com>
Cc: Tatyana Krasnukha <tatyana at synopsys.com>; lldb-commits <lldb-commits at lists.llvm.org>
Subject: Re: [Lldb-commits] [lldb] r355406 - Revert "Fix embedded Python initialization according to changes in version 3.7"

On Tue, Mar 5, 2019 at 8:39 AM Stella Stamenova <stilis at microsoft.com> wrote:
>
> The bot is running 3.6.6 and it does not have python 3.7 installed (it does have python 2.7 which is needed to run Buildbot).
>
> The change that was pushed, though, did not only impact python 3.7:
>
>                 m_was_already_initialized == PyGILState_UNLOCKED ? "un" : "");
>                 m_gil_state == PyGILState_UNLOCKED ? "un" : "");
>
> It replaced m_was_already_initialized above with m_gil_state - for *any* version of python. Yes, this is just in a log, but my guess is that that's the source of the hang on Windows not the 3.7 specific code.
>
> If you end up with a  prototype change that you want to commit, I can run a local test to see whether the tests pass with it.
>

Can you just revert that chunk of unconditional python code and try?
(it's extremely surprising to me that the change caused the bots to hang, even if it's unconditional, anyway).

i.e.

+// Since Python 3.7 `Py_Initialize` calls `PyEval_InitThreads` inside 
+itself, // so there is no way to determine whether the embedded 
+interpreter // was already initialized by some external code.
+`PyEval_ThreadsInitialized` // would always return `true` and 
+`PyGILState_Ensure/Release` flow would be // executed instead of 
+unlocking GIL with `PyEval_SaveThread`. When // an another thread calls `PyGILState_Ensure` it would get stuck in deadlock.
+#if (PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION >= 7) || 
+(PY_MAJOR_VERSION > 3) // The only case we should go further and acquire the GIL: it is unlocked.
+    if (PyGILState_Check())
+      return;
+#endif
+

and let us know if the bot still hangs?

Thanks,

--
Davide