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 <dccitali...@gmail.com> Sent: Tuesday, March 5, 2019 8:43 AM To: Stella Stamenova <sti...@microsoft.com> Cc: Tatyana Krasnukha <taty...@synopsys.com>; lldb-commits <lldb-commits@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 <sti...@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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
