Force propertynames(::Py) to occur on the main thread (#677)

* Force propertynames(::Py) to occur on the main thread

* Refactor, document, make more robust and remove a lock

* Make setup_onfixedthread robust against errors

* Make on_main_thread test more reliable

---------

Co-authored-by: Christopher Doris <github.com/cjdoris>

Author: Liozou

src/C/context.jl

@@ -29,6 +29,79 @@ function _atpyexit()
     return
 end
 
+
+function setup_onfixedthread()
+    channel_input = Channel(1)
+    channel_output = Channel(1)
+    islaunched = Ref(false) # use Ref to avoid closure boxing of variable
+    function launch_worker(tid)
+        islaunched[] && error("Cannot launch more than once: call setup_onfixedthread again if need be.")
+        islaunched[] = true
+        worker_task = Task() do
+            while true
+                f = take!(channel_input)
+                ret = try
+                    Some(invokelatest(f))
+                    # invokelatest is necessary for development and interactive use.
+                    # Otherwise, only a method f defined in a world prior to the call of
+                    # launch_worker would work.
+                catch e
+                    e, catch_backtrace()
+                end
+                put!(channel_output, ret)
+            end
+        end
+        # code adapted from set_task_tid! in StableTasks.jl, itself taken from Dagger.jl
+        worker_task.sticky = true
+        for _ in 1:100
+            # try to fix the task id to tid, retrying up to 100 times
+            ret = ccall(:jl_set_task_tid, Cint, (Any, Cint), worker_task, tid-1)
+            if ret == 1
+                break # success
+            elseif ret == 0
+                yield()
+            else
+                error("Unexpected retcode from jl_set_task_tid: $ret")
+            end
+        end
+        if Threads.threadid(worker_task) != tid
+            error("Failed setting the thread ID to $tid.")
+        end
+        schedule(worker_task)
+    end
+    function onfixedthread(f)
+        put!(channel_input, f)
+        ret = take!(channel_output)
+        if ret isa Tuple
+            e, backtrace = ret
+            printstyled(stderr, "ERROR: "; color=:red, bold=true)
+            showerror(stderr, e)
+            Base.show_backtrace(stderr, backtrace)
+            println(stderr)
+            throw(e) # the stacktrace of the actual error is printed above
+        else
+            something(ret)
+        end
+    end
+    launch_worker, onfixedthread
+end
+
+# launch_on_main_thread is used in init_context(), after which on_main_thread becomes usable
+const launch_on_main_thread, on_main_thread = setup_onfixedthread()
+
+"""
+    on_main_thread(f)
+
+Execute `f()` on the main thread.
+
+!!! warning
+    The value returned by `on_main_thread(f)` cannot be type-inferred by the compiler:
+    if necessary, use explicit type annotations such as `on_main_thread(f)::T`, where `T` is
+    the expected return type.
+"""
+on_main_thread
+
+
 function init_context()
 
     CTX.is_embedded = hasproperty(Base.Main, :__PythonCall_libptr)
@@ -240,6 +313,8 @@ function init_context()
         "Only Python 3.10+ is supported, this is Python $(CTX.version) at $(CTX.exe_path===missing ? "unknown location" : CTX.exe_path).",
     )
 
+    launch_on_main_thread(Threads.threadid()) # makes on_main_thread usable
+
     @debug "Initialized PythonCall.jl" CTX.is_embedded CTX.is_initialized CTX.exe_path CTX.lib_path CTX.lib_ptr CTX.pyprogname CTX.pyhome CTX.version
 
     return

src/Core/Py.jl

@@ -282,25 +282,29 @@ Base.setproperty!(x::Py, k::Symbol, v) = pysetattr(x, string(k), v)
 Base.setproperty!(x::Py, k::String, v) = pysetattr(x, k, v)
 
 function Base.propertynames(x::Py, private::Bool = false)
-    # this follows the logic of rlcompleter.py
-    function classmembers(c)
-        r = pydir(c)
-        if pyhasattr(c, "__bases__")
-            for b in c.__bases__
-                r = pyiadd(r, classmembers(b))
+    properties = C.on_main_thread() do
+        # this follows the logic of rlcompleter.py
+        function classmembers(c)
+            r = pydir(c)
+            if pyhasattr(c, "__bases__")
+                for b in c.__bases__
+                    r = pyiadd(r, classmembers(b))
+                end
             end
+            return r
         end
-        return r
-    end
-    words = pyset(pydir(x))
-    words.discard("__builtins__")
-    if pyhasattr(x, "__class__")
-        words.add("__class__")
-        words.update(classmembers(x.__class__))
-    end
-    words = map(pystr_asstring, words)
+
+        words = pyset(pydir(x::Py))
+        words.discard("__builtins__")
+        if pyhasattr(x, "__class__")
+            words.add("__class__")
+            words.update(classmembers(x.__class__))
+        end
+        map(pystr_asstring, words)
+    end::Vector{String} # explicit type since on_main_thread() is type-unstable
+
     # private || filter!(w->!startswith(w, "_"), words)
-    map(Symbol, words)
+    map(Symbol, properties)
 end
 
 Base.Bool(x::Py) = pytruth(x)

test/Core.jl

@@ -830,3 +830,23 @@ end
         @test !isdir(tname)
     end
 end
+
+@testitem "propertynames" begin
+    x = pyint(7)
+    task = Threads.@spawn propertynames(x)
+    properties = propertynames(x)
+    @test :__init__ in properties
+    prop_task = fetch(task)
+    @test properties == prop_task
+end
+
+@testitem "on_main_thread" begin
+    refid = PythonCall.C.on_main_thread() do; Threads.threadid(); end
+    tasks = [Threads.@spawn(PythonCall.C.on_main_thread() do; Threads.threadid(); end) for _ in 1:20]
+    @test all(t -> fetch(t) == refid, tasks)
+    @test_throws DivideError redirect_stderr(devnull) do
+        PythonCall.C.on_main_thread() do
+            throw(DivideError())
+        end
+    end
+end