Merge pull request #1334 from kmod/threading

Threading fixes to get cffi-1.7 working

from_cpython/Include/pystate.h

@@ -20,12 +20,13 @@ typedef struct _is {
     struct _is *next;
     struct _ts *tstate_head;
 
+    PyObject *modules;
+    PyObject *builtins;
+
     // Pyston change
     // Note: any changes here need to show up in PyInterpreterState_Clear as well
 #if 0
-    PyObject *modules;
     PyObject *sysdict;
-    PyObject *builtins;
     PyObject *modules_reloading;
 
     PyObject *codec_search_path;
@@ -136,10 +137,7 @@ PyAPI_FUNC(int) PyThreadState_SetAsyncExc(long, PyObject *) PYSTON_NOEXCEPT;
 
 /* Variable and macro for in-line access to current thread state */
 
-// Pyston change: use our internal name for this
-//PyAPI_DATA(PyThreadState *) _PyThreadState_Current;
-PyAPI_DATA(__thread PyThreadState) cur_thread_state;
-#define _PyThreadState_Current (&cur_thread_state)
+PyAPI_DATA(PyThreadState *) _PyThreadState_Current;
 
 #ifdef Py_DEBUG
 #define PyThreadState_GET() PyThreadState_Get()

from_cpython/Python/import.c

@@ -3164,6 +3164,17 @@ imp_load_dynamic(PyObject *self, PyObject *args)
     if (!PyArg_ParseTuple(args, "ss|O!:load_dynamic", &name, &pathname,
                           &PyFile_Type, &fob))
         return NULL;
+
+    // Pyston change: warn if it doesn't look like a pyston-built so.
+    // Some libraries (cffi) want to load things with names that don't even end
+    // in ".so", so allow those through.
+    int len = strlen(name);
+    if (len >= 3 && strcmp(&name[len - 3], ".so") == 0 &&
+        (len < 10 || strcmp(&name[len - 10], ".pyston.so") != 0)) {
+        PyErr_Format(PyExc_ValueError, "Pyston refusing to load a non-pyston .so \"%.200s\"", name);
+        return NULL;
+    }
+
     if (fob) {
         fp = get_file(pathname, fob, "r");
         if (fp == NULL)

src/codegen/ast_interpreter.cpp

@@ -1664,11 +1664,11 @@ Value ASTInterpreter::visit_dict(AST_Dict* node) {
 }
 
 Value ASTInterpreter::visit_set(AST_Set* node) {
+    BoxedSet* set = (BoxedSet*)createSet();
     try {
         // insert the elements in reverse like cpython does
         // important for {1, 1L}
         llvm::SmallVector<RewriterVar*, 8> items;
-        BoxedSet* set = (BoxedSet*)createSet();
         for (auto it = node->elts.rbegin(), it_end = node->elts.rend(); it != it_end; ++it) {
             Value v = visit_expr(*it);
             _setAddStolen(set, v.o);
@@ -1676,7 +1676,8 @@ Value ASTInterpreter::visit_set(AST_Set* node) {
         }
         return Value(set, jit ? jit->emitCreateSet(items) : NULL);
     } catch (ExcInfo e) {
-        RELEASE_ASSERT(0, "this leaks in case of an exception");
+        Py_DECREF(set);
+        throw e;
     }
 }
 

src/codegen/codegen.cpp

@@ -37,8 +37,6 @@
 
 namespace pyston {
 
-DS_DEFINE_RWLOCK(codegen_rwlock);
-
 FunctionMetadata::FunctionMetadata(int num_args, bool takes_varargs, bool takes_kwargs,
                                    std::unique_ptr<SourceInfo> source)
     : code_obj(NULL),

src/codegen/codegen.h

@@ -90,8 +90,6 @@ extern GlobalState g;
 
 // in runtime_hooks.cpp:
 void initGlobalFuncs(GlobalState& g);
-
-DS_DECLARE_RWLOCK(codegen_rwlock);
 }
 
 #endif

src/codegen/irgen/hooks.cpp

@@ -341,30 +341,26 @@ CompiledFunction* compileFunction(FunctionMetadata* f, FunctionSpecialization* s
 void compileAndRunModule(AST_Module* m, BoxedModule* bm) {
     FunctionMetadata* md;
 
-    { // scope for limiting the locked region:
-        LOCK_REGION(codegen_rwlock.asWrite());
+    Timer _t("for compileModule()");
 
-        Timer _t("for compileModule()");
+    const char* fn = PyModule_GetFilename(bm);
+    RELEASE_ASSERT(fn, "");
 
-        const char* fn = PyModule_GetFilename(bm);
-        RELEASE_ASSERT(fn, "");
+    FutureFlags future_flags = getFutureFlags(m->body, fn);
+    ScopingAnalysis* scoping = new ScopingAnalysis(m, true);
 
-        FutureFlags future_flags = getFutureFlags(m->body, fn);
-        ScopingAnalysis* scoping = new ScopingAnalysis(m, true);
+    auto fn_str = boxString(fn);
+    AUTO_DECREF(fn_str);
+    std::unique_ptr<SourceInfo> si(new SourceInfo(bm, scoping, future_flags, m, fn_str));
 
-        auto fn_str = boxString(fn);
-        AUTO_DECREF(fn_str);
-        std::unique_ptr<SourceInfo> si(new SourceInfo(bm, scoping, future_flags, m, fn_str));
+    static BoxedString* doc_str = getStaticString("__doc__");
+    bm->setattr(doc_str, autoDecref(si->getDocString()), NULL);
 
-        static BoxedString* doc_str = getStaticString("__doc__");
-        bm->setattr(doc_str, autoDecref(si->getDocString()), NULL);
+    static BoxedString* builtins_str = getStaticString("__builtins__");
+    if (!bm->hasattr(builtins_str))
+        bm->setattr(builtins_str, PyModule_GetDict(builtins_module), NULL);
 
-        static BoxedString* builtins_str = getStaticString("__builtins__");
-        if (!bm->hasattr(builtins_str))
-            bm->setattr(builtins_str, PyModule_GetDict(builtins_module), NULL);
-
-        md = new FunctionMetadata(0, false, false, std::move(si));
-    }
+    md = new FunctionMetadata(0, false, false, std::move(si));
 
     UNAVOIDABLE_STAT_TIMER(t0, "us_timer_interpreted_module_toplevel");
     Box* r = astInterpretFunction(md, NULL, NULL, NULL, NULL, NULL, NULL, NULL);
@@ -395,8 +391,6 @@ Box* evalOrExec(FunctionMetadata* md, Box* globals, Box* boxedLocals) {
 
 static FunctionMetadata* compileForEvalOrExec(AST* source, llvm::ArrayRef<AST_stmt*> body, BoxedString* fn,
                                               PyCompilerFlags* flags) {
-    LOCK_REGION(codegen_rwlock.asWrite());
-
     Timer _t("for evalOrExec()");
 
     ScopingAnalysis* scoping = new ScopingAnalysis(source, false);
@@ -655,8 +649,6 @@ void exec(Box* boxedCode, Box* globals, Box* locals, FutureFlags caller_future_f
 // TODO we should have logic like this at the CLFunc level that detects that we keep
 // on creating functions with failing speculations, and then stop speculating.
 void CompiledFunction::speculationFailed() {
-    LOCK_REGION(codegen_rwlock.asWrite());
-
     this->times_speculation_failed++;
 
     if (this->times_speculation_failed == 4) {
@@ -735,8 +727,6 @@ ConcreteCompilerType* CompiledFunction::getReturnType() {
 /// The cf must be an active version in its parents FunctionMetadata; the given
 /// version will be replaced by the new version, which will be returned.
 static CompiledFunction* _doReopt(CompiledFunction* cf, EffortLevel new_effort) {
-    LOCK_REGION(codegen_rwlock.asWrite());
-
     assert(cf->md->versions.size());
 
     assert(cf);
@@ -773,8 +763,6 @@ static CompiledFunction* _doReopt(CompiledFunction* cf, EffortLevel new_effort)
 static StatCounter stat_osrexits("num_osr_exits");
 static StatCounter stat_osr_compiles("num_osr_compiles");
 CompiledFunction* compilePartialFuncInternal(OSRExit* exit) {
-    LOCK_REGION(codegen_rwlock.asWrite());
-
     assert(exit);
     stat_osrexits.log();
 

src/core/threading.h

@@ -21,6 +21,8 @@
 #include <ucontext.h>
 #include <vector>
 
+#include "Python.h"
+
 #include "core/common.h"
 #include "core/thread_utils.h"
 
@@ -33,6 +35,8 @@ extern int sigprof_pending;
 void _printStacktrace();
 #endif
 
+extern __thread PyThreadState cur_thread_state;
+
 namespace threading {
 
 // Whether or not a second thread was ever started:
@@ -47,35 +51,10 @@ void finishMainThread();
 
 bool isMainThread();
 
-#ifndef THREADING_USE_GIL
-#define THREADING_USE_GIL 1
-#define THREADING_USE_GRWL 0
-#endif
-#define THREADING_SAFE_DATASTRUCTURES THREADING_USE_GRWL
-
-#if THREADING_SAFE_DATASTRUCTURES
-#define DS_DEFINE_MUTEX(name) pyston::threading::PthreadFastMutex name
-
-#define DS_DECLARE_RWLOCK(name) extern pyston::threading::PthreadRWLock name
-#define DS_DEFINE_RWLOCK(name) pyston::threading::PthreadRWLock name
-
-#define DS_DEFINE_SPINLOCK(name) pyston::threading::PthreadSpinLock name
-#else
-#define DS_DEFINE_MUTEX(name) pyston::threading::NopLock name
-
-#define DS_DECLARE_RWLOCK(name) extern pyston::threading::NopLock name
-#define DS_DEFINE_RWLOCK(name) pyston::threading::NopLock name
-
-#define DS_DEFINE_SPINLOCK(name) pyston::threading::NopLock name
-#endif
-
-void acquireGLRead();
-void releaseGLRead();
-void acquireGLWrite();
-void releaseGLWrite();
 void _allowGLReadPreemption();
 
 #define GIL_CHECK_INTERVAL 1000
+
 // Note: this doesn't need to be an atomic, since it should
 // only be accessed by the thread that holds the gil:
 extern int gil_check_count;
@@ -104,26 +83,8 @@ extern "C" inline void allowGLReadPreemption() {
 
     _allowGLReadPreemption();
 }
-// Note: promoteGL is free to drop the lock and then reacquire
-void promoteGL();
-void demoteGL();
-
 
 
-// Helper macro for creating a RAII wrapper around two functions.
-#define MAKE_REGION(name, start, end)                                                                                  \
-    class name {                                                                                                       \
-    public:                                                                                                            \
-        name() { start(); }                                                                                            \
-        ~name() { end(); }                                                                                             \
-    };
-
-MAKE_REGION(GLReadRegion, acquireGLRead, releaseGLRead);
-MAKE_REGION(GLPromoteRegion, promoteGL, demoteGL);
-// MAKE_REGION(GLReadReleaseRegion, releaseGLRead, acquireGLRead);
-// MAKE_REGION(GLWriteReleaseRegion, releaseGLWrite, acquireGLWrite);
-#undef MAKE_REGION
-
 extern "C" void beginAllowThreads() noexcept;
 extern "C" void endAllowThreads() noexcept;
 
@@ -134,37 +95,6 @@ public:
 };
 
 
-#if THREADING_USE_GIL
-inline void acquireGLRead() {
-    acquireGLWrite();
-}
-inline void releaseGLRead() {
-    releaseGLWrite();
-}
-inline void promoteGL() {
-}
-inline void demoteGL() {
-}
-#endif
-
-#if !THREADING_USE_GIL && !THREADING_USE_GRWL
-inline void acquireGLRead() {
-}
-inline void releaseGLRead() {
-}
-inline void acquireGLWrite() {
-}
-inline void releaseGLWrite() {
-}
-inline void promoteGL() {
-}
-inline void demoteGL() {
-}
-extern "C" inline void allowGLReadPreemption() __attribute__((visibility("default")));
-extern "C" inline void allowGLReadPreemption() {
-}
-#endif
-
 extern bool forgot_refs_via_fork;
 
 } // namespace threading

src/jit.cpp

@@ -369,7 +369,6 @@ static int main(int argc, char** argv) noexcept {
         const char* fn = NULL;
 
         threading::registerMainThread();
-        threading::acquireGLRead();
 
         Py_SetProgramName(argv[0]);
 
@@ -540,11 +539,6 @@ static int main(int argc, char** argv) noexcept {
 
         threading::finishMainThread();
 
-        // Acquire the GIL to make sure we stop the other threads, since we will tear down
-        // data structures they are potentially running on.
-        // Note: we will purposefully not release the GIL on exiting.
-        threading::promoteGL();
-
         _t.split("Py_Finalize");
 
         Py_Finalize();

src/runtime/builtin_modules/builtins.cpp

@@ -2376,6 +2376,7 @@ void setupBuiltins() {
     builtins_module = createModule(autoDecref(boxString("__builtin__")), NULL,
                                    "Built-in functions, exceptions, and other objects.\n\nNoteworthy: None is "
                                    "the `nil' object; Ellipsis represents `...' in slices.");
+    PyThreadState_GET()->interp->builtins = incref(builtins_module->getAttrWrapper());
 
     ellipsis_cls
         = BoxedClass::create(type_cls, object_cls, 0, 0, sizeof(Box), false, "ellipsis", false, NULL, NULL, false);

src/runtime/builtin_modules/sys.cpp

@@ -795,6 +795,8 @@ static int _check_and_flush(FILE* stream) {
 
 void setupSys() {
     sys_modules_dict = new BoxedDict();
+    PyThreadState_GET()->interp->modules = incref(sys_modules_dict);
+
     constants.push_back(sys_modules_dict);
 
     // This is ok to call here because we've already created the sys_modules_dict

src/runtime/capi.cpp

@@ -1483,31 +1483,6 @@ extern "C" void PyEval_InitThreads(void) noexcept {
     // nothing to do here
 }
 
-extern "C" void PyEval_AcquireThread(PyThreadState* tstate) noexcept {
-    Py_FatalError("Unimplemented");
-}
-
-extern "C" void PyEval_ReleaseThread(PyThreadState* tstate) noexcept {
-    Py_FatalError("Unimplemented");
-}
-
-extern "C" PyThreadState* PyThreadState_Get(void) noexcept {
-    if (_PyThreadState_Current == NULL)
-        Py_FatalError("PyThreadState_Get: no current thread");
-
-    return _PyThreadState_Current;
-}
-
-extern "C" PyThreadState* PyEval_SaveThread(void) noexcept {
-    beginAllowThreads();
-    return PyThreadState_GET();
-}
-
-extern "C" void PyEval_RestoreThread(PyThreadState* tstate) noexcept {
-    RELEASE_ASSERT(tstate == PyThreadState_GET(), "");
-    endAllowThreads();
-}
-
 extern "C" BORROWED(struct _frame*) PyEval_GetFrame(void) noexcept {
     Box* frame = NULL;
     try {

src/runtime/import.cpp

@@ -139,8 +139,9 @@ extern "C" Box* import(int level, Box* from_imports, llvm::StringRef module_name
 BoxedModule* importCExtension(BoxedString* full_name, const std::string& last_name, const std::string& path) {
     void* handle = dlopen(path.c_str(), RTLD_NOW);
     if (!handle) {
+        const char* s = dlerror();
         // raiseExcHelper(ImportError, "%s", dlerror());
-        fprintf(stderr, "%s\n", dlerror());
+        fprintf(stderr, "%s\n", s);
         exit(1);
     }
     assert(handle);

src/runtime/objmodel.cpp

@@ -3967,8 +3967,6 @@ static StatCounter slowpath_pickversion("slowpath_pickversion");
 template <ExceptionStyle S>
 static CompiledFunction* pickVersion(FunctionMetadata* f, int num_output_args, Box* oarg1, Box* oarg2, Box* oarg3,
                                      Box** oargs) {
-    LOCK_REGION(codegen_rwlock.asWrite());
-
     // if always_use_version is set use it even if the exception style does not match.
     // But prefer using the correct style if both are available
     if (f->always_use_version.get(S))

test/extra/cffi_1.7_test.py

+# Note: the expected counts here are set to match the CI, and I can't reproduce them locally
+import os, sys, subprocess, shutil
+sys.path.append(os.path.dirname(__file__) + "/../lib")
+
+from test_helper import create_virtenv, run_test
+
+ENV_NAME = "cffi17_test_env_" + os.path.basename(sys.executable)
+SRC_DIR = os.path.abspath(os.path.join(ENV_NAME, "src"))
+PYTHON_EXE = os.path.abspath(os.path.join(ENV_NAME, "bin", "python"))
+PYTEST_EXE = os.path.abspath(os.path.join(ENV_NAME, "bin", "py.test"))
+
+def install_and_test_cffi():
+    shutil.rmtree(SRC_DIR, ignore_errors=True)
+    os.makedirs(SRC_DIR)
+
+    url = "https://pypi.python.org/packages/83/3c/00b553fd05ae32f27b3637f705c413c4ce71290aa9b4c4764df694e906d9/cffi-1.7.0.tar.gz#md5=34122a545060cee58bab88feab57006d"
+    subprocess.check_call(["wget", url], cwd=SRC_DIR)
+    subprocess.check_call(["tar", "-zxf", "cffi-1.7.0.tar.gz"], cwd=SRC_DIR)
+    CFFI_DIR = os.path.abspath(os.path.join(SRC_DIR, "cffi-1.7.0"))
+
+    subprocess.check_call([PYTHON_EXE, "setup.py", "install"], cwd=CFFI_DIR)
+
+    # looks like clang 3.5 causes more errors like: 214 != -42 doing casts
+    if os.environ.has_key("CC") and "clang" in os.environ["CC"]:
+        expected = [{'xfailed': 4, 'failed': 3, 'skipped': 10, 'passed': 539}]
+    else:
+        expected = [{'xfailed': 4, 'failed': 2, 'skipped': 10, 'passed': 540}]
+
+    # Ideally we would run all the tests, but they take ~10min to run.
+    # dir_to_test = "."
+    # I just picked a subdirectory; I don't really know what it's testing.
+    dir_to_test = os.path.join(CFFI_DIR,  "testing", "cffi1")
+    run_test([PYTEST_EXE, dir_to_test], cwd=CFFI_DIR, expected=expected)
+
+create_virtenv(ENV_NAME, ["pytest==2.8.7", "py==1.4.31", "pycparser==2.14"], force_create = True)
+install_and_test_cffi()

test/extra/cffi_test.py

-# Note: the expected counts here are set to match the CI, and I can't re[rpduce them locally
+# Note: the expected counts here are set to match the CI, and I can't reproduce them locally
 import os, sys, subprocess, shutil
 sys.path.append(os.path.dirname(__file__) + "/../lib")
 
@@ -29,6 +29,6 @@ def install_and_test_cffi():
     else:
         expected = [{ "failed": 11, "passed": 1668, "skipped": 73, "xfailed": 4}]
     run_test([PYTEST_EXE], cwd=CFFI_DIR, expected=expected)
-   
+
 create_virtenv(ENV_NAME, ["pytest==2.8.7", "py==1.4.31", "pycparser==2.14"], force_create = True)
 install_and_test_cffi()

test/lib/test_helper.py

@@ -66,6 +66,8 @@ def run_test(cmd, cwd, expected, env = None):
 
     print
     print "Return code:", errcode
+    assert errcode in (0, 1), "\n\n%s\nTest process crashed" % output
+
     if expected == result:
         print "Received expected output"
     else:

test/tests/set.expected

@@ -540,3 +540,4 @@ False
 set([1L]) set([1]) set([1]) set([1L])
 set([1])
 4
+unhashable type: 'dict'

test/tests/set.py

@@ -274,3 +274,8 @@ from api_test import set_size
 
 s = set([1, 2, 3, 4])
 print(set_size(s))
+
+try:
+    {{}}
+except Exception as e:
+    print e

test/unittests/unittests.h

@@ -25,12 +25,10 @@ namespace pyston {
 class PystonTestEnvironment : public testing::Environment {
     void SetUp() override {
         threading::registerMainThread();
-
-        threading::acquireGLRead();
     }
 
     void TearDown() override {
-        threading::releaseGLRead();
+        // threading::releaseGLRead();
     }
 };