[Lldb-commits] [lldb] r162161 - in /lldb/trunk: ./ include/lldb/Interpreter/ScriptInterpreterPython.h scripts/Python/build-swig-Python.sh source/Expression/ClangUserExpression.cpp source/Interpreter/ScriptInterpreterPython.cpp

Johnny Chen johnny.chen at apple.com
Mon Aug 20 10:02:26 PDT 2012 

Hi Filipe,

Thank you for contributing to the python GIL work, among other things, of course. :-)

On Aug 20, 2012, at 3:42 AM, Filipe Cabecinhas <filcab at gmail.com> wrote:

> Thanks! 
> 
> I will be contributing some more Python stuff this week, now that we have GIL support :)
> 
> Regards, 
> 
>  Filipe
> 
> 
> On Saturday, August 18, 2012 at 5:14 AM, Johnny Chen wrote:
> 
>> Author: johnny
>> Date: Fri Aug 17 23:14:54 2012
>> New Revision: 162161
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=162161&view=rev
>> Log:
>> Merge python-GIL bracnh (by filcab) back into trunk!
>> 
>> Modified:
>> lldb/trunk/ (props changed)
>> lldb/trunk/include/lldb/Interpreter/ScriptInterpreterPython.h
>> lldb/trunk/scripts/Python/build-swig-Python.sh (http://build-swig-Python.sh)
>> lldb/trunk/source/Expression/ClangUserExpression.cpp
>> lldb/trunk/source/Interpreter/ScriptInterpreterPython.cpp
>> 
>> Propchange: lldb/trunk/
>> ------------------------------------------------------------------------------
>> svn:mergeinfo = /lldb/branches/apple/python-GIL:156467-162159
>> 
>> Modified: lldb/trunk/include/lldb/Interpreter/ScriptInterpreterPython.h
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/ScriptInterpreterPython.h?rev=162161&r1=162160&r2=162161&view=diff
>> ==============================================================================
>> --- lldb/trunk/include/lldb/Interpreter/ScriptInterpreterPython.h (original)
>> +++ lldb/trunk/include/lldb/Interpreter/ScriptInterpreterPython.h Fri Aug 17 23:14:54 2012
>> @@ -253,10 +253,7 @@
>> FILE* wait_msg_handle = NULL);
>> 
>> ~Locker ();
>> - 
>> - static bool
>> - CurrentThreadHasPythonLock ();
>> - 
>> +
>> private:
>> 
>> bool
>> @@ -270,17 +267,14 @@
>> 
>> bool
>> DoTearDownSession ();
>> - 
>> - static bool
>> - TryGetPythonLock (uint32_t seconds_to_wait);
>> - 
>> +
>> static void
>> ReleasePythonLock ();
>> 
>> bool m_need_session;
>> - bool m_release_lock;
>> ScriptInterpreterPython *m_python_interpreter;
>> FILE* m_tmp_fh;
>> + PyGILState_STATE m_GILState;
>> };
>> 
>> class PythonInputReaderManager
>> @@ -335,7 +329,6 @@
>> bool m_session_is_active;
>> bool m_pty_slave_is_open;
>> bool m_valid_session;
>> - 
>> };
>> } // namespace lldb_private
>> 
>> 
>> Modified: lldb/trunk/scripts/Python/build-swig-Python.sh (http://build-swig-Python.sh)
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/Python/build-swig-Python.sh?rev=162161&r1=162160&r2=162161&view=diff
>> ==============================================================================
>> --- lldb/trunk/scripts/Python/build-swig-Python.sh (http://build-swig-Python.sh) (original)
>> +++ lldb/trunk/scripts/Python/build-swig-Python.sh (http://build-swig-Python.sh) Fri Aug 17 23:14:54 2012
>> @@ -289,7 +289,7 @@
>> 
>> # Build the SWIG C++ wrapper file for Python.
>> 
>> -$SWIG -c++ -shadow -python -I"/usr/include" -I"${SRC_ROOT}/include" -I./. -outdir "${CONFIG_BUILD_DIR}" -o "${swig_output_file}" "${swig_input_file}"
>> +$SWIG -c++ -shadow -python -threads -I"/usr/include" -I"${SRC_ROOT}/include" -I./. -outdir "${CONFIG_BUILD_DIR}" -o "${swig_output_file}" "${swig_input_file}"
>> 
>> # Implement the iterator protocol and/or eq/ne operators for some lldb objects.
>> # Append global variable to lldb Python module.
>> 
>> Modified: lldb/trunk/source/Expression/ClangUserExpression.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/ClangUserExpression.cpp?rev=162161&r1=162160&r2=162161&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Expression/ClangUserExpression.cpp (original)
>> +++ lldb/trunk/source/Expression/ClangUserExpression.cpp Fri Aug 17 23:14:54 2012
>> @@ -556,8 +556,10 @@
>> lldb::addr_t object_ptr = 0;
>> lldb::addr_t cmd_ptr = 0;
>> 
>> - if (!PrepareToExecuteJITExpression (error_stream, exe_ctx, struct_address, object_ptr, cmd_ptr))
>> + if (!PrepareToExecuteJITExpression (error_stream, exe_ctx, struct_address, object_ptr, cmd_ptr)) {
>> + error_stream.Printf("Errored out in %s, couldn't PrepareToExecuteJITExpression", __FUNCTION__);
>> return eExecutionSetupError;
>> + }
>> 
>> const bool stop_others = true;
>> const bool try_all_threads = true;
>> @@ -630,6 +632,7 @@
>> if (FinalizeJITExecution (error_stream, exe_ctx, result, function_stack_pointer))
>> return eExecutionCompleted;
>> else
>> + error_stream.Printf("Errored out in %s: Couldn't FinalizeJITExpression", __FUNCTION__);
>> return eExecutionSetupError;
>> }
>> else
>> 
>> Modified: lldb/trunk/source/Interpreter/ScriptInterpreterPython.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/ScriptInterpreterPython.cpp?rev=162161&r1=162160&r2=162161&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Interpreter/ScriptInterpreterPython.cpp (original)
>> +++ lldb/trunk/source/Interpreter/ScriptInterpreterPython.cpp Fri Aug 17 23:14:54 2012
>> @@ -128,73 +128,18 @@
>> return fflush (stream) || prev_fail ? EOF : 0;
>> }
>> 
>> -static Predicate<lldb::tid_t> &
>> -PythonMutexPredicate ()
>> -{
>> - static lldb_private::Predicate<lldb::tid_t> g_interpreter_is_running (LLDB_INVALID_THREAD_ID);
>> - return g_interpreter_is_running;
>> -}
>> -
>> -bool
>> -ScriptInterpreterPython::Locker::CurrentThreadHasPythonLock ()
>> -{
>> - TimeValue timeout;
>> -
>> - timeout = TimeValue::Now(); // Don't wait any time.
>> -
>> - return PythonMutexPredicate().WaitForValueEqualTo (Host::GetCurrentThreadID(), &timeout, NULL);
>> -}
>> -
>> -bool
>> -ScriptInterpreterPython::Locker::TryGetPythonLock (uint32_t seconds_to_wait)
>> -{
>> - 
>> - TimeValue timeout;
>> - 
>> - if (seconds_to_wait != UINT32_MAX)
>> - {
>> - timeout = TimeValue::Now();
>> - timeout.OffsetWithSeconds (seconds_to_wait);
>> - }
>> - 
>> - return PythonMutexPredicate().WaitForValueEqualToAndSetValueTo (LLDB_INVALID_THREAD_ID, 
>> - Host::GetCurrentThreadID(), &timeout, NULL);
>> -}
>> -
>> -void
>> -ScriptInterpreterPython::Locker::ReleasePythonLock ()
>> -{
>> - PythonMutexPredicate().SetValue (LLDB_INVALID_THREAD_ID, eBroadcastAlways);
>> -}
>> -
>> ScriptInterpreterPython::Locker::Locker (ScriptInterpreterPython *py_interpreter,
>> uint16_t on_entry,
>> uint16_t on_leave,
>> FILE* wait_msg_handle) :
>> m_need_session( (on_leave & TearDownSession) == TearDownSession ),
>> - m_release_lock ( false ), // decide in constructor body
>> m_python_interpreter(py_interpreter),
>> m_tmp_fh(wait_msg_handle)
>> {
>> if (m_python_interpreter && !m_tmp_fh)
>> m_tmp_fh = (m_python_interpreter->m_dbg_stdout ? m_python_interpreter->m_dbg_stdout : stdout);
>> - 
>> - if ( (on_entry & AcquireLock) == AcquireLock )
>> - {
>> - if (CurrentThreadHasPythonLock())
>> - {
>> - if ( (on_leave & FreeLock) == FreeLock )
>> - m_release_lock = true;
>> - }
>> - else
>> - {
>> - DoAcquireLock();
>> - if ( (on_leave & FreeLock) == FreeLock )
>> - m_release_lock = true;
>> - if ( (on_leave & FreeAcquiredLock) == FreeAcquiredLock )
>> - m_release_lock = true;
>> - }
>> - }
>> +
>> + DoAcquireLock();
>> if ( (on_entry & InitSession) == InitSession )
>> DoInitSession();
>> }
>> @@ -202,13 +147,10 @@
>> bool
>> ScriptInterpreterPython::Locker::DoAcquireLock()
>> {
>> - if (!CurrentThreadHasPythonLock())
>> - {
>> - while (!TryGetPythonLock (1))
>> - if (m_tmp_fh)
>> - fprintf (m_tmp_fh, 
>> - "Python interpreter locked on another thread; waiting to acquire lock...\n");
>> - }
>> + LogSP log (lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_SCRIPT | LIBLLDB_LOG_VERBOSE));
>> + m_GILState = PyGILState_Ensure();
>> + if (log)
>> + log->Printf("Ensured PyGILState. Previous state = %slocked\n", m_GILState == PyGILState_UNLOCKED ? "un" : "");
>> return true;
>> }
>> 
>> @@ -224,7 +166,10 @@
>> bool
>> ScriptInterpreterPython::Locker::DoFreeLock()
>> {
>> - ReleasePythonLock ();
>> + LogSP log (lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_SCRIPT | LIBLLDB_LOG_VERBOSE));
>> + if (log)
>> + log->Printf("Releasing PyGILState. Returning to state = %slocked\n", m_GILState == PyGILState_UNLOCKED ? "un" : "");
>> + PyGILState_Release(m_GILState);
>> return true;
>> }
>> 
>> @@ -241,8 +186,7 @@
>> {
>> if (m_need_session)
>> DoTearDownSession();
>> - if (m_release_lock)
>> - DoFreeLock();
>> + DoFreeLock();
>> }
>> 
>> class ForceDisableSyntheticChildren
>> @@ -357,13 +301,7 @@
>> input_fd = STDIN_FILENO;
>> 
>> script_interpreter->SaveTerminalState(input_fd);
>> - 
>> - {
>> - ScriptInterpreterPython::Locker locker(script_interpreter,
>> - ScriptInterpreterPython::Locker::AcquireLock | ScriptInterpreterPython::Locker::InitSession,
>> - ScriptInterpreterPython::Locker::FreeAcquiredLock);
>> - }
>> - 
>> +
>> char error_str[1024];
>> if (script_interpreter->m_embedded_thread_pty.OpenFirstAvailableMaster (O_RDWR|O_NOCTTY, error_str, 
>> sizeof(error_str)))
>> @@ -377,6 +315,9 @@
>> const char *pty_slave_name = script_interpreter->m_embedded_thread_pty.GetSlaveName (error_str, sizeof (error_str));
>> if (pty_slave_name != NULL && PyThreadState_GetDict() != NULL)
>> {
>> + ScriptInterpreterPython::Locker locker(script_interpreter,
>> + ScriptInterpreterPython::Locker::AcquireLock | ScriptInterpreterPython::Locker::InitSession,
>> + ScriptInterpreterPython::Locker::FreeAcquiredLock);
>> run_string.Printf ("run_one_line (%s, 'save_stderr = sys.stderr')", script_interpreter->m_dictionary_name.c_str());
>> PyRun_SimpleString (run_string.GetData());
>> run_string.Clear ();
>> @@ -428,20 +369,9 @@
>> 
>> case eInputReaderReactivate:
>> {
>> - // Don't try and acquire the interpreter lock here because code like
>> - // this:
>> - //
>> - // (lldb) script
>> - // >>> v = lldb.frame.EvaluateExpression("collection->get_at_index(12)")
>> - //
>> - // This will cause the process to run. The interpreter lock is taken
>> - // by the input reader for the "script" command. If we try and acquire
>> - // the lock here, when the process runs it might deactivate this input
>> - // reader (if STDIN is hooked up to the inferior process) and 
>> - // reactivate it when the process stops which will deadlock.
>> - //ScriptInterpreterPython::Locker locker(script_interpreter,
>> - // ScriptInterpreterPython::Locker::AcquireLock | ScriptInterpreterPython::Locker::InitSession,
>> - // ScriptInterpreterPython::Locker::FreeAcquiredLock);
>> + ScriptInterpreterPython::Locker locker(script_interpreter,
>> + ScriptInterpreterPython::Locker::AcquireLock | ScriptInterpreterPython::Locker::InitSession,
>> + ScriptInterpreterPython::Locker::FreeAcquiredLock);
>> }
>> break;
>> 
>> @@ -484,6 +414,9 @@
>> const char *pty_slave_name = script_interpreter->m_embedded_thread_pty.GetSlaveName (error_str, sizeof (error_str));
>> if (pty_slave_name != NULL && PyThreadState_GetDict() != NULL)
>> {
>> + ScriptInterpreterPython::Locker locker(script_interpreter,
>> + ScriptInterpreterPython::Locker::AcquireLock | ScriptInterpreterPython::Locker::InitSession,
>> + ScriptInterpreterPython::Locker::FreeAcquiredLock);
>> run_string.Printf ("run_one_line (%s, 'sys.stdin = save_stdin')", script_interpreter->m_dictionary_name.c_str());
>> PyRun_SimpleString (run_string.GetData());
>> run_string.Clear();
>> @@ -531,14 +464,14 @@
>> g_initialized = true;
>> ScriptInterpreterPython::InitializePrivate ();
>> }
>> - 
>> - Locker locker(this,
>> - ScriptInterpreterPython::Locker::AcquireLock,
>> - ScriptInterpreterPython::Locker::FreeAcquiredLock);
>> 
>> m_dictionary_name.append("_dict");
>> StreamString run_string;
>> run_string.Printf ("%s = dict()", m_dictionary_name.c_str());
>> +
>> + Locker locker(this,
>> + ScriptInterpreterPython::Locker::AcquireLock,
>> + ScriptInterpreterPython::Locker::FreeAcquiredLock);
>> PyRun_SimpleString (run_string.GetData());
>> 
>> run_string.Clear();
>> @@ -957,20 +890,9 @@
>> 
>> case eInputReaderReactivate:
>> {
>> - // Don't try and acquire the interpreter lock here because code like
>> - // this:
>> - //
>> - // (lldb) script
>> - // >>> v = lldb.frame.EvaluateExpression("collection->get_at_index(12)")
>> - //
>> - // This will cause the process to run. The interpreter lock is taken
>> - // by the input reader for the "script" command. If we try and acquire
>> - // the lock here, when the process runs it might deactivate this input
>> - // reader (if STDIN is hooked up to the inferior process) and 
>> - // reactivate it when the process stops which will deadlock.
>> - //ScriptInterpreterPython::Locker locker(script_interpreter,
>> - // ScriptInterpreterPython::Locker::AcquireLock | ScriptInterpreterPython::Locker::InitSession,
>> - // ScriptInterpreterPython::Locker::FreeAcquiredLock);
>> + ScriptInterpreterPython::Locker locker(script_interpreter,
>> + ScriptInterpreterPython::Locker::AcquireLock | ScriptInterpreterPython::Locker::InitSession,
>> + ScriptInterpreterPython::Locker::FreeAcquiredLock);
>> }
>> break;
>> 
>> @@ -1012,7 +934,12 @@
>> break;
>> 
>> case eInputReaderDone:
>> - script_interpreter->LeaveSession ();
>> + {
>> + Locker locker(script_interpreter,
>> + ScriptInterpreterPython::Locker::AcquireLock,
>> + ScriptInterpreterPython::Locker::FreeAcquiredLock);
>> + script_interpreter->LeaveSession ();
>> + }
>> 
>> // Restore terminal settings if they were validly saved
>> if (log)
>> @@ -2033,14 +1960,18 @@
>> char error_str[1024];
>> const char *pty_slave_name = script_interpreter->m_embedded_python_pty.GetSlaveName (error_str, sizeof (error_str));
>> 
>> - Locker locker(script_interpreter,
>> - ScriptInterpreterPython::Locker::AcquireLock | ScriptInterpreterPython::Locker::InitSession,
>> - ScriptInterpreterPython::Locker::FreeAcquiredLock | ScriptInterpreterPython::Locker::TearDownSession);
>> -
>> if (pty_slave_name != NULL)
>> {
>> StreamString run_string;
>> - 
>> +
>> + // Ensure we have the GIL before running any Python code.
>> + // Since we're only running a few one-liners and then dropping to the interpreter (which will release the GIL when needed),
>> + // we can just release the GIL after finishing our work.
>> + // If finer-grained locking is desirable, we can lock and unlock the GIL only when calling a python function.
>> + Locker locker(script_interpreter,
>> + ScriptInterpreterPython::Locker::AcquireLock | ScriptInterpreterPython::Locker::InitSession,
>> + ScriptInterpreterPython::Locker::FreeAcquiredLock | ScriptInterpreterPython::Locker::TearDownSession);
>> +
>> run_string.Printf ("run_one_line (%s, 'save_stderr = sys.stderr')", script_interpreter->m_dictionary_name.c_str());
>> PyRun_SimpleString (run_string.GetData());
>> run_string.Clear ();
>> @@ -2060,36 +1991,25 @@
>> 
>> // The following call drops into the embedded interpreter loop and stays there until the
>> // user chooses to exit from the Python interpreter.
>> + // This embedded interpreter will, as any Python code that performs I/O, unlock the GIL before
>> + // a system call that can hang, and lock it when the syscall has returned.
>> 
>> - // When in the embedded interpreter, the user can call arbitrary system and Python stuff, which may require
>> - // the ability to run multi-threaded stuff, so we need to surround the call to the embedded interpreter with
>> - // calls to Py_BEGIN_ALLOW_THREADS and Py_END_ALLOW_THREADS.
>> -
>> - // We ALSO need to surround the call to the embedded interpreter with calls to PyGILState_Ensure and 
>> - // PyGILState_Release. This is because this embedded interpreter is being run on a DIFFERENT THREAD than
>> - // the thread on which the call to Py_Initialize (and PyEval_InitThreads) was called. Those initializations 
>> - // called PyGILState_Ensure on *that* thread, but it also needs to be called on *this* thread. Otherwise,
>> - // if the user calls Python code that does threading stuff, the interpreter state will be off, and things could
>> - // hang (it's happened before).
>> + // We need to surround the call to the embedded interpreter with calls to PyGILState_Ensure and 
>> + // PyGILState_Release (using the Locker above). This is because Python has a global lock which must be held whenever we want
>> + // to touch any Python objects. Otherwise, if the user calls Python code, the interpreter state will be off,
>> + // and things could hang (it's happened before).
>> 
>> - Py_BEGIN_ALLOW_THREADS
>> - PyGILState_STATE gstate = PyGILState_Ensure();
>> - 
>> run_string.Printf ("run_python_interpreter (%s)", script_interpreter->m_dictionary_name.c_str());
>> PyRun_SimpleString (run_string.GetData());
>> run_string.Clear ();
>> - 
>> - PyGILState_Release (gstate);
>> - Py_END_ALLOW_THREADS
>> - 
>> +
>> run_string.Printf ("run_one_line (%s, 'sys.stdin = save_stdin')", script_interpreter->m_dictionary_name.c_str());
>> PyRun_SimpleString (run_string.GetData());
>> run_string.Clear();
>> - 
>> +
>> run_string.Printf ("run_one_line (%s, 'sys.stderr = save_stderr')", script_interpreter->m_dictionary_name.c_str());
>> PyRun_SimpleString (run_string.GetData());
>> run_string.Clear();
>> - 
>> }
>> 
>> if (script_interpreter->m_embedded_python_input_reader_sp)
>> @@ -2261,8 +2181,6 @@
>> lldb::DebuggerSP debugger_sp = m_interpreter.GetDebugger().shared_from_this();
>> 
>> {
>> - Locker py_lock(this);
>> - 
>> FileSpec target_file(pathname, true);
>> 
>> // TODO: would we want to reject any other value?
>> @@ -2275,6 +2193,9 @@
>> 
>> const char* directory = target_file.GetDirectory().GetCString();
>> std::string basename(target_file.GetFilename().GetCString());
>> +
>> + // Before executing Pyton code, lock the GIL.
>> + Locker py_lock(this);
>> 
>> // now make sure that Python has "directory" in the search path
>> StreamString command_stream;
>> @@ -2457,7 +2378,18 @@
>> TerminalState stdin_tty_state;
>> stdin_tty_state.Save(STDIN_FILENO, false);
>> 
>> - PyEval_InitThreads ();
>> + PyGILState_STATE gstate;
>> + LogSP log (lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_SCRIPT | LIBLLDB_LOG_VERBOSE));
>> + bool threads_already_initialized = false;
>> + if (PyEval_ThreadsInitialized ()) {
>> + gstate = PyGILState_Ensure ();
>> + if (log)
>> + log->Printf("Ensured PyGILState. Previous state = %slocked\n", gstate == PyGILState_UNLOCKED ? "un" : "");
>> + threads_already_initialized = true;
>> + } else {
>> + // InitThreads acquires the GIL if it hasn't been called before.
>> + PyEval_InitThreads ();
>> + }
>> Py_InitializeEx (0);
>> 
>> // Initialize SWIG after setting up python
>> @@ -2500,6 +2432,15 @@
>> 
>> PyRun_SimpleString ("sys.dont_write_bytecode = 1; import lldb.embedded_interpreter; from lldb.embedded_interpreter import run_python_interpreter; from lldb.embedded_interpreter import run_one_line; from termios import *");
>> 
>> + if (threads_already_initialized) {
>> + if (log)
>> + log->Printf("Releasing PyGILState. Returning to state = %slocked\n", gstate == PyGILState_UNLOCKED ? "un" : "");
>> + PyGILState_Release (gstate);
>> + } else {
>> + // We initialized the threads in this function, just unlock the GIL.
>> + PyEval_SaveThread();
>> + }
>> +
>> stdin_tty_state.Restore();
>> }
>> 
>> 
>> 
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at cs.uiuc.edu (mailto:lldb-commits at cs.uiuc.edu)
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> 
> 
>

More information about the lldb-commits mailing list