make Profile more thread/signal-safe

Fixes #35117
Fixes #13294

Author: vtjnash

base/process.jl

@@ -546,7 +546,7 @@ Returns successfully if the process has already exited, but throws an
 error if killing the process failed for other reasons (e.g. insufficient
 permissions).
 """
-function kill(p::Process, signum::Integer)
+function kill(p::Process, signum::Integer=SIGTERM)
     iolock_begin()
     if process_running(p)
         @assert p.handle != C_NULL
@@ -558,9 +558,8 @@ function kill(p::Process, signum::Integer)
     iolock_end()
     nothing
 end
-kill(ps::Vector{Process}) = foreach(kill, ps)
-kill(ps::ProcessChain) = foreach(kill, ps.processes)
-kill(p::Process) = kill(p, SIGTERM)
+kill(ps::Vector{Process}, signum::Integer=SIGTERM) = for p in ps; kill(p, signum); end
+kill(ps::ProcessChain, signum::Integer=SIGTERM) = kill(ps.processes, signum)
 
 """
     getpid(process) -> Int32

src/debuginfo.cpp

@@ -23,8 +23,6 @@
 
 using namespace llvm;
 
-using llvm_file_magic = file_magic;
-
 #include "julia.h"
 #include "julia_internal.h"
 #include "debuginfo.h"
@@ -52,11 +50,41 @@ typedef object::SymbolRef SymRef;
 // and cannot have any interaction with the julia runtime
 static uv_rwlock_t threadsafe;
 
-extern "C" void jl_init_debuginfo()
+extern "C" void jl_init_debuginfo(void)
 {
     uv_rwlock_init(&threadsafe);
 }
 
+extern "C" void jl_lock_profile(void)
+{
+    uv_rwlock_rdlock(&threadsafe);
+}
+
+extern "C" void jl_unlock_profile(void)
+{
+    uv_rwlock_rdunlock(&threadsafe);
+}
+
+// some actions aren't signal (especially profiler) safe so we acquire a lock
+// around them to establish a mutual exclusion with unwinding from a signal
+template <typename T>
+static void jl_profile_atomic(T f)
+{
+    uv_rwlock_wrlock(&threadsafe);
+#ifndef _OS_WINDOWS_
+    sigset_t sset;
+    sigset_t oset;
+    sigfillset(&sset);
+    pthread_sigmask(SIG_BLOCK, &sset, &oset);
+#endif
+    f();
+#ifndef _OS_WINDOWS_
+    pthread_sigmask(SIG_SETMASK, &oset, NULL);
+#endif
+    uv_rwlock_wrunlock(&threadsafe);
+}
+
+
 // --- storing and accessing source location metadata ---
 
 struct ObjectInfo {
@@ -131,13 +159,15 @@ static void create_PRUNTIME_FUNCTION(uint8_t *Code, size_t Size, StringRef fnnam
         JL_UNLOCK_NOGC(&jl_in_stackwalk);
     }
 #if defined(_CPU_X86_64_)
-    if (!RtlAddFunctionTable(tbl, 1, (DWORD64)Section)) {
-        static int warned = 0;
-        if (!warned) {
-            jl_printf(JL_STDERR, "WARNING: failed to insert function stack unwind info: %lu\n", GetLastError());
-            warned = 1;
+    jl_profile_atomic([&]() {
+        if (!RtlAddFunctionTable(tbl, 1, (DWORD64)Section)) {
+            static int warned = 0;
+            if (!warned) {
+                jl_printf(JL_STDERR, "WARNING: failed to insert function stack unwind info: %lu\n", GetLastError());
+                warned = 1;
+            }
         }
-    }
+    });
 #endif
 }
 #endif
@@ -278,7 +308,9 @@ class JuliaJITEventListener: public JITEventListener
             di->u.rti.name_ptr = 0;
             di->u.rti.table_data = arm_exidx_addr;
             di->u.rti.table_len = arm_exidx_len;
-            _U_dyn_register(di);
+            jl_profile_atomic([&]() {
+                _U_dyn_register(di);
+            });
             break;
         }
 #endif
@@ -404,20 +436,20 @@ class JuliaJITEventListener: public JITEventListener
                 codeinst = codeinst_it->second;
                 codeinst_in_flight.erase(codeinst_it);
             }
-            uv_rwlock_wrlock(&threadsafe);
-            if (codeinst)
-                linfomap[Addr] = std::make_pair(Size, codeinst->def);
-            if (first) {
-                ObjectInfo tmp = {&debugObj,
-                    (size_t)SectionSize,
-                    (ptrdiff_t)(SectionAddr - SectionLoadAddr),
-                    *Section,
-                    nullptr,
-                    };
-                objectmap[SectionLoadAddr] = tmp;
-                first = false;
-            }
-            uv_rwlock_wrunlock(&threadsafe);
+            jl_profile_atomic([&]() {
+                if (codeinst)
+                    linfomap[Addr] = std::make_pair(Size, codeinst->def);
+                if (first) {
+                    ObjectInfo tmp = {&debugObj,
+                        (size_t)SectionSize,
+                        (ptrdiff_t)(SectionAddr - SectionLoadAddr),
+                        *Section,
+                        nullptr,
+                        };
+                    objectmap[SectionLoadAddr] = tmp;
+                    first = false;
+                }
+            });
         }
         jl_gc_safe_leave(ptls, gc_state);
     }
@@ -431,14 +463,6 @@ class JuliaJITEventListener: public JITEventListener
         uv_rwlock_rdlock(&threadsafe);
         return objectmap;
     }
-
-    Optional<std::map<size_t, ObjectInfo, revcomp>*> trygetObjectMap()
-    {
-        if (0 == uv_rwlock_tryrdlock(&threadsafe)) {
-            return &objectmap;
-        }
-        return {};
-    }
 };
 
 JL_DLLEXPORT void ORCNotifyObjectEmitted(JITEventListener *Listener,
@@ -482,7 +506,7 @@ static std::pair<char *, bool> jl_demangle(const char *name) JL_NOTSAFEPOINT
 }
 
 static JuliaJITEventListener *jl_jit_events;
-JITEventListener *CreateJuliaJITEventListener()
+JITEventListener *CreateJuliaJITEventListener(void)
 {
     jl_jit_events = new JuliaJITEventListener();
     return jl_jit_events;
@@ -722,7 +746,7 @@ openDebugInfo(StringRef debuginfopath, const debug_link_info &info)
 
     auto error_splitobj = object::ObjectFile::createObjectFile(
             SplitFile.get().get()->getMemBufferRef(),
-            llvm_file_magic::unknown);
+            file_magic::unknown);
     if (!error_splitobj) {
         return error_splitobj.takeError();
     }
@@ -873,7 +897,7 @@ static objfileentry_t &find_object_file(uint64_t fbase, StringRef fname) JL_NOTS
         std::unique_ptr<MemoryBuffer> membuf = MemoryBuffer::getMemBuffer(
                 StringRef((const char *)fbase, msize), "", false);
         auto origerrorobj = llvm::object::ObjectFile::createObjectFile(
-            membuf->getMemBufferRef(), llvm_file_magic::unknown);
+            membuf->getMemBufferRef(), file_magic::unknown);
         if (!origerrorobj)
             return entry;
 
@@ -1292,28 +1316,33 @@ void register_eh_frames(uint8_t *Addr, size_t Size)
   // See http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-April/061768.html
   processFDEs((char*)Addr, Size, [](const char *Entry) {
         if (!libc_register_frame) {
-          libc_register_frame = (void(*)(void*))dlsym(RTLD_NEXT,"__register_frame");
+          libc_register_frame = (void(*)(void*))dlsym(RTLD_NEXT, "__register_frame");
         }
         assert(libc_register_frame);
-        libc_register_frame(const_cast<char *>(Entry));
-        __register_frame(const_cast<char *>(Entry));
+        jl_profile_atomic([&]() {
+            libc_register_frame(const_cast<char *>(Entry));
+            __register_frame(const_cast<char *>(Entry));
+        });
     });
 }
 
 void deregister_eh_frames(uint8_t *Addr, size_t Size)
 {
    processFDEs((char*)Addr, Size, [](const char *Entry) {
         if (!libc_deregister_frame) {
-          libc_deregister_frame = (void(*)(void*))dlsym(RTLD_NEXT,"__deregister_frame");
+          libc_deregister_frame = (void(*)(void*))dlsym(RTLD_NEXT, "__deregister_frame");
         }
         assert(libc_deregister_frame);
-        libc_deregister_frame(const_cast<char *>(Entry));
-        __deregister_frame(const_cast<char *>(Entry));
+        jl_profile_atomic([&]() {
+            libc_deregister_frame(const_cast<char *>(Entry));
+            __deregister_frame(const_cast<char *>(Entry));
+        });
     });
 }
 
 #elif defined(_OS_LINUX_) && \
-    defined(JL_UNW_HAS_FORMAT_IP) && !defined(_CPU_ARM_)
+    defined(JL_UNW_HAS_FORMAT_IP) && \
+    !defined(_CPU_ARM_) // ARM does not have/use .eh_frame, so we handle this elsewhere
 #include <type_traits>
 
 struct unw_table_entry
@@ -1499,7 +1528,9 @@ static DW_EH_PE parseCIE(const uint8_t *Addr, const uint8_t *End)
 void register_eh_frames(uint8_t *Addr, size_t Size)
 {
     // System unwinder
-    __register_frame(Addr);
+    jl_profile_atomic([&]() {
+        __register_frame(Addr);
+    });
     // Our unwinder
     unw_dyn_info_t *di = new unw_dyn_info_t;
     // In a shared library, this is set to the address of the PLT.
@@ -1610,7 +1641,7 @@ void register_eh_frames(uint8_t *Addr, size_t Size)
             start_ips[cur_entry] = start;
             cur_entry++;
         });
-    for (size_t i = 0;i < nentries;i++) {
+    for (size_t i = 0; i < nentries; i++) {
         table[i].start_ip_offset =
             safe_trunc<int32_t>((intptr_t)start_ips[i] - (intptr_t)start_ip);
     }
@@ -1621,27 +1652,21 @@ void register_eh_frames(uint8_t *Addr, size_t Size)
     di->start_ip = start_ip;
     di->end_ip = end_ip;
 
-    _U_dyn_register(di);
+    jl_profile_atomic([&]() {
+        _U_dyn_register(di);
+    });
 }
 
 void deregister_eh_frames(uint8_t *Addr, size_t Size)
 {
-    __deregister_frame(Addr);
-    // Deregistering with our unwinder requires a lookup table to find the
-    // the allocated entry above (or we could look in libunwind's internal
+    jl_profile_atomic([&]() {
+        __deregister_frame(Addr);
+    });
+    // Deregistering with our unwinder (_U_dyn_cancel) requires a lookup table
+    // to find the allocated entry above (or looking into libunwind's internal
     // data structures).
 }
 
-#elif defined(_CPU_ARM_)
-
-void register_eh_frames(uint8_t *Addr, size_t Size)
-{
-}
-
-void deregister_eh_frames(uint8_t *Addr, size_t Size)
-{
-}
-
 #else
 
 void register_eh_frames(uint8_t *Addr, size_t Size)
@@ -1667,22 +1692,3 @@ uint64_t jl_getUnwindInfo(uint64_t dwAddr)
     uv_rwlock_rdunlock(&threadsafe);
     return ipstart;
 }
-
-extern "C"
-uint64_t jl_trygetUnwindInfo(uint64_t dwAddr)
-{
-    // Might be called from unmanaged thread
-    Optional<std::map<size_t, ObjectInfo, revcomp>*> maybeobjmap = jl_jit_events->trygetObjectMap();
-    if (maybeobjmap) {
-        std::map<size_t, ObjectInfo, revcomp> &objmap = **maybeobjmap;
-        std::map<size_t, ObjectInfo, revcomp>::iterator it = objmap.lower_bound(dwAddr);
-        uint64_t ipstart = 0; // ip of the start of the section (if found)
-        if (it != objmap.end() && dwAddr < it->first + it->second.SectionSize) {
-            ipstart = (uint64_t)(uintptr_t)(*it).first;
-        }
-        uv_rwlock_rdunlock(&threadsafe);
-        return ipstart;
-    }
-    return 0;
-}
-

src/julia_internal.h

@@ -756,7 +756,6 @@ typedef struct {
 
 // Might be called from unmanaged thread
 uint64_t jl_getUnwindInfo(uint64_t dwBase);
-uint64_t jl_trygetUnwindInfo(uint64_t dwBase);
 #ifdef _OS_WINDOWS_
 #include <dbghelp.h>
 JL_DLLEXPORT EXCEPTION_DISPOSITION __julia_personality(

src/signal-handling.c

@@ -27,6 +27,8 @@ static const    uint64_t GIGA = 1000000000ULL;
 // Timers to take samples at intervals
 JL_DLLEXPORT void jl_profile_stop_timer(void);
 JL_DLLEXPORT int jl_profile_start_timer(void);
+void jl_lock_profile(void);
+void jl_unlock_profile(void);
 
 static uint64_t jl_last_sigint_trigger = 0;
 static uint64_t jl_disable_sigint_time = 0;

src/signals-mach.c

@@ -462,6 +462,7 @@ void *mach_profile_listener(void *arg)
         HANDLE_MACH_ERROR("mach_msg", ret);
         // sample each thread, round-robin style in reverse order
         // (so that thread zero gets notified last)
+        jl_lock_profile();
         for (i = jl_n_threads; i-- > 0; ) {
             // if there is no space left, break early
             if (bt_size_cur >= bt_size_max - 1)
@@ -506,14 +507,16 @@ void *mach_profile_listener(void *arg)
 
                 // Mark the end of this block with 0
                 bt_data_prof[bt_size_cur++].uintptr = 0;
-
-                // Reset the alarm
-                kern_return_t ret = clock_alarm(clk, TIME_RELATIVE, timerprof, profile_port);
-                HANDLE_MACH_ERROR("clock_alarm", ret)
             }
             // We're done! Resume the thread.
             jl_thread_resume(i, 0);
         }
+        jl_unlock_profile();
+        if (running) {
+            // Reset the alarm
+            kern_return_t ret = clock_alarm(clk, TIME_RELATIVE, timerprof, profile_port);
+            HANDLE_MACH_ERROR("clock_alarm", ret)
+        }
     }
 }
 
@@ -547,6 +550,7 @@ JL_DLLEXPORT int jl_profile_start_timer(void)
     timerprof.tv_nsec = nsecprof%GIGA;
 
     running = 1;
+    // ensure the alarm is running
     ret = clock_alarm(clk, TIME_RELATIVE, timerprof, profile_port);
     HANDLE_MACH_ERROR("clock_alarm", ret);
 

src/signals-unix.c

@@ -673,6 +673,8 @@ static void *signal_listener(void *arg)
         unw_context_t *signal_context;
         // sample each thread, round-robin style in reverse order
         // (so that thread zero gets notified last)
+        if (critical || profile)
+            jl_lock_profile();
         for (int i = jl_n_threads; i-- > 0; ) {
             // notify thread to stop
             jl_thread_suspend_and_get_state(i, &signal_context);
@@ -717,6 +719,8 @@ static void *signal_listener(void *arg)
             // notify thread to resume
             jl_thread_resume(i, sig);
         }
+        if (critical || profile)
+            jl_unlock_profile();
 #endif
 
         // this part is async with the running of the rest of the program