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

Tue Mar 5 08:39:19 PST 2019

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.

Thanks,
-Stella

-----Original Message-----
From: Davide Italiano <dccitaliano at gmail.com> 
Sent: Tuesday, March 5, 2019 8:26 AM
To: Tatyana Krasnukha <tatyana at synopsys.com>; Stella Stamenova <stilis at microsoft.com>
Cc: 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"

After the revert the testsuite doesn't hang anymore, so, it looks like this was responsible. Stella, is the bot actually running python 3.6 ?
Can you make sure there's no python 3.7 installed somewhere in the system that gets picked by accident?

On Tue, Mar 5, 2019 at 7:59 AM Davide Italiano <dccitaliano at gmail.com> wrote:
>
> I'm also at a loss trying to understand how this particular change 
> could've caused the tests to hang.
>
>   __VSCMD_PREINIT_PATH=C:\Program Files (x86)\Microsoft Visual 
> Studio\Shared\Python36_64\Scripts\;C:\Program Files (x86)\Microsoft 
> Visual 
> Studio\Shared\Python36_64\;C:\Windows\system32;C:\Windows;C:\Windows\S
> ystem32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Program
> Files\dotnet\;C:\Program Files (x86)\GtkSharp\2.12\bin;C:\Program
> Files\CMake\bin;C:\Program Files (x86)\swigwin-3.0.12;C:\Program Files 
> (x86)\GetGnuWin32\gnuwin32\bin;c:\Python27\Scripts\;C:\Program Files 
> (x86)\Ninja;C:\Program Files\Git\cmd;C:\Program
> Files\TortoiseSVN\bin;C:\Users\buildslave\AppData\Local\Microsoft\Wind
> owsApps;C:\Python27\lib\site-packages\pywin32_system32;C:\Python27\lib
> \site-packages\pywin32_system32
>  using PTY: False
>
>
> This is what I see on the Windows bot, so, I expect that it runs 
> Python 3.6, but the whole fix is conditional to PY_MAJOR_VERSION == 3 
> && PY_MINOR_VERSION >= 7.
>
> On Tue, Mar 5, 2019 at 7:53 AM Davide Italiano <dccitaliano at gmail.com> wrote:
> >
> > This is unfortunate, because I think it's the correct path forward.
> > Stella, do you know why this is failing on Windows?
> >
> > Thanks,
> >
> > --
> > Davide
> >
> >
> > On Tue, Mar 5, 2019 at 7:26 AM Tatyana Krasnukha via lldb-commits 
> > <lldb-commits at lists.llvm.org> wrote:
> > >
> > > Author: tkrasnukha
> > > Date: Tue Mar  5 07:27:33 2019
> > > New Revision: 355406
> > >
> > > URL: 
> > > https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fl
> > > lvm.org%2Fviewvc%2Fllvm-project%3Frev%3D355406%26view%3Drev&da
> > > ta=02%7C01%7CSTILIS%40microsoft.com%7Cc10a6a537cd64f69e2cb08d6a187
> > > 4c51%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6368739998069545
> > > 86&sdata=lVokkbUu5XHoInVa4sYLT90X%2Fqj%2BJOcWcUkSD%2BdCyDQ%3D&
> > > amp;reserved=0
> > > Log:
> > > Revert "Fix embedded Python initialization according to changes in version 3.7"
> > >
> > > Testsuite hangs on Windows likely due to these changes.
> > >
> > > Modified:
> > >     
> > > lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpret
> > > erPython.cpp
> > >
> > > Modified: 
> > > lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpret
> > > erPython.cpp
> > > URL: 
> > > https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fl
> > > lvm.org%2Fviewvc%2Fllvm-project%2Flldb%2Ftrunk%2Fsource%2FPlugins%
> > > 2FScriptInterpreter%2FPython%2FScriptInterpreterPython.cpp%3Frev%3
> > > D355406%26r1%3D355405%26r2%3D355406%26view%3Ddiff&data=02%7C01
> > > %7CSTILIS%40microsoft.com%7Cc10a6a537cd64f69e2cb08d6a1874c51%7C72f
> > > 988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636873999806954586&sda
> > > ta=WOgZzE1py846x%2FJjvG%2F3SqCwXWOfMMqBtzS22GUITCw%3D&reserved
> > > =0 
> > > ==================================================================
> > > ============
> > > --- 
> > > lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpret
> > > erPython.cpp (original)
> > > +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInter
> > > +++ preterPython.cpp Tue Mar  5 07:27:33 2019
> > > @@ -156,7 +156,7 @@ public:
> > >      if (m_was_already_initialized) {
> > >        Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_SCRIPT));
> > >        LLDB_LOGV(log, "Releasing PyGILState. Returning to state = {0}locked",
> > > -                m_gil_state == PyGILState_UNLOCKED ? "un" : "");
> > > +                m_was_already_initialized == PyGILState_UNLOCKED 
> > > + ? "un" : "");
> > >        PyGILState_Release(m_gil_state);
> > >      } else {
> > >        // We initialized the threads in this function, just unlock the GIL.
> > > @@ -180,18 +180,6 @@ private:
> > >    }
> > >
> > >    void InitializeThreadsPrivate() { -// 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
> > > -
> > >      if (PyEval_ThreadsInitialized()) {
> > >        Log 
> > > *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_SCRIPT));
> > >
> > >
> > >
> > > _______________________________________________
> > > lldb-commits mailing list
> > > lldb-commits at lists.llvm.org
> > > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > > lists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Flldb-commits&d
> > > ata=02%7C01%7CSTILIS%40microsoft.com%7Cc10a6a537cd64f69e2cb08d6a18
> > > 74c51%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636873999806954
> > > 586&sdata=6RUlUG%2FnKoMrpvZRX15u46G4cjAwKuZq7%2B8ikTltntE%3D&a
> > > mp;reserved=0