pystack - pypi Package Compare versions

Comparing version 1.2.0 to 1.3.0

build_scripts/elfutils_contiguous_segments.diff

+diff --git a/libdwfl/dwfl_segment_report_module.c b/libdwfl/dwfl_segment_report_module.c
+index 3ef62a7d..09ee37b3 100644
+--- a/libdwfl/dwfl_segment_report_module.c
++++ b/libdwfl/dwfl_segment_report_module.c
+@@ -737,17 +737,34 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
+ 	        && invalid_elf (module->elf, module->disk_file_has_build_id,
+ 				&build_id))
+ 	      {
+-		elf_end (module->elf);
+-		close (module->fd);
+-		module->elf = NULL;
+-		module->fd = -1;
++		/* If MODULE's build-id doesn't match the disk file's
++		   build-id, close ELF only if MODULE and ELF refer to
++		   different builds of files with the same name.  This
++		   prevents premature closure of the correct ELF in cases
++		   where segments of a module are non-contiguous in memory.  */
++		if (name != NULL && module->name[0] != '\0'
++		    && strcmp (basename (module->name), basename (name)) == 0)
++		  {
++		    elf_end (module->elf);
++		    close (module->fd);
++		    module->elf = NULL;
++		    module->fd = -1;
++		  }
+ 	      }
+-	    if (module->elf != NULL)
++	    else if (module->elf != NULL)
+ 	      {
+-		/* Ignore this found module if it would conflict in address
+-		   space with any already existing module of DWFL.  */
++		/* This module has already been reported.  */
+ 		skip_this_module = true;
+ 	      }
++	    else
++	      {
++		/* Only report this module if we haven't already done so.  */
++		for (Dwfl_Module *mod = dwfl->modulelist; mod != NULL;
++		     mod = mod->next)
++		  if (mod->low_addr == module_start
++		      && mod->high_addr == module_end)
++		    skip_this_module = true;
++	      }
+ 	  }
+       if (skip_this_module)
+ 	goto out;
+@@ -781,10 +798,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
+ 	}
+     }
+
+-  /* Our return value now says to skip the segments contained
+-     within the module.  */
+-  ndx = addr_segndx (dwfl, segment, module_end, true);
+-
+   /* Examine its .dynamic section to get more interesting details.
+      If it has DT_SONAME, we'll use that as the module name.
+      If it has a DT_DEBUG, then it's actually a PIE rather than a DSO.
+@@ -929,6 +942,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
+       ndx = -1;
+       goto out;
+     }
++  else
++    ndx++;
+
+   /* We have reported the module.  Now let the caller decide whether we
+      should read the whole thing in right now.  */

.bumpversion.cfg

 [bumpversion]
-current_version = 1.2.0
+current_version = 1.3.0
 commit = True
@@ -4,0 +4,0 @@ message =

MANIFEST.in

@@ -17,2 +17,3 @@ exclude .clang-format
 include .pre-commit-config.yaml
+include build_scripts/elfutils_contiguous_segments.diff
 
@@ -19,0 +20,0 @@ recursive-include src/pystack/_pystack *

NEWS.rst

@@ -11,2 +11,15 @@ .. note
 
+pystack 1.3.0 (2023-11-28)
+--------------------------
+
+Bug Fixes
+~~~~~~~~~
+
+- Add a patch to the bundled elfutils used to create wheels to account for a bug when analysing cores with interleaved segments (#153)
+- Removed the unused ``--self`` flag. (#141)
+- Fix some instances when identifying the pthread id was failing in systems without GLIBC (#152)
+- Fix several some race conditions when stopping threads in multithreaded programs (#155)
+- Ensure log messages that contain non-UTF-8 data are not lost (#155)
+
+
 pystack 1.2.0 (2023-07-31)
@@ -13,0 +26,0 @@ --------------------------

PKG-INFO

 Metadata-Version: 2.1
 Name: pystack
-Version: 1.2.0
+Version: 1.3.0
 Summary: Analysis of the stack of remote python processes
@@ -91,2 +91,4 @@ Home-page: https://github.com/bloomberg/pystack
 compiler where to find the header and library files of the dependencies for the build to succeed.
+If `pkg-config` is available (e.g. `apt-get install pkg-config` on Debian-based systems), it will
+automatically be used to locate the libraries and configure the correct build flags.
 Check your distribution's documentation to determine the location of the header and library files
@@ -156,3 +158,3 @@ or for more detailed information. When building on Alpine Linux (or any other distribution that
 ```shell
-usage: pystack remote [-h] [-v] [--no-color] [--no-block] [--native] [--native-all] [--locals] [--exhaustive] [--self] pid
+usage: pystack remote [-h] [-v] [--no-color] [--no-block] [--native] [--native-all] [--locals] [--exhaustive] pid
 
@@ -159,0 +161,0 @@ positional arguments:

pyproject.toml

@@ -6,3 +6,4 @@ [build-system]
      "wheel",
-     "Cython"
+     "Cython",
+     "pkgconfig"
 ]
@@ -59,2 +60,3 @@
   "cd elfutils-$VERS",
+  "patch libdwfl/dwfl_segment_report_module.c < {package}/build_scripts/elfutils_contiguous_segments.diff",
   "CFLAGS='-w' ./configure --enable-libdebuginfod=dummy --disable-debuginfod --prefix=/usr --libdir=/usr/lib64",
@@ -97,2 +99,3 @@ "make install"
   "cd elfutils-$VERS",
+  "patch libdwfl/dwfl_segment_report_module.c < {package}/build_scripts/elfutils_contiguous_segments.diff",
   "CFLAGS='-w -DFNM_EXTMATCH=0' ./configure --prefix=/usr --disable-nls --disable-libdebuginfod --disable-debuginfod --with-zstd",
@@ -99,0 +102,0 @@ "make install"

README.md

@@ -71,2 +71,4 @@ <p align="center">
 compiler where to find the header and library files of the dependencies for the build to succeed.
+If `pkg-config` is available (e.g. `apt-get install pkg-config` on Debian-based systems), it will
+automatically be used to locate the libraries and configure the correct build flags.
 Check your distribution's documentation to determine the location of the header and library files
@@ -136,3 +138,3 @@ or for more detailed information. When building on Alpine Linux (or any other distribution that
 ```shell
-usage: pystack remote [-h] [-v] [--no-color] [--no-block] [--native] [--native-all] [--locals] [--exhaustive] [--self] pid
+usage: pystack remote [-h] [-v] [--no-color] [--no-block] [--native] [--native-all] [--locals] [--exhaustive] pid
 
@@ -139,0 +141,0 @@ positional arguments:

setup.py

@@ -6,2 +6,3 @@ import os
 
+import pkgconfig
 import setuptools
@@ -13,3 +14,3 @@ from Cython.Build import cythonize
 if not IS_LINUX:
-    raise RuntimeError(f"memray does not support this platform ({platform})")
+    raise RuntimeError(f"pystack does not support this platform ({platform})")
 
@@ -57,3 +58,13 @@ install_requires = []
 
+library_flags = {"libraries": ["elf", "dw"]}
 
+try:
+    library_flags = pkgconfig.parse("libelf libdw")
+except EnvironmentError as e:
+    print("pkg-config not found.", e)
+    print("Falling back to static flags.")
+except pkgconfig.PackageNotFoundError as e:
+    print("Package Not Found", e)
+    print("Falling back to static flags.")
+
 PYSTACK_EXTENSION = setuptools.Extension(
@@ -75,4 +86,2 @@ name="pystack._pystack",
     ],
-    libraries=["elf", "dw"],
-    include_dirs=["src"],
     language="c++",
@@ -82,2 +91,3 @@ extra_compile_args=["-std=c++17"],
     define_macros=DEFINE_MACROS,
+    **library_flags,
 )
@@ -84,0 +94,0 @@

src/pystack.egg-info/PKG-INFO

 Metadata-Version: 2.1
 Name: pystack
-Version: 1.2.0
+Version: 1.3.0
 Summary: Analysis of the stack of remote python processes
@@ -91,2 +91,4 @@ Home-page: https://github.com/bloomberg/pystack
 compiler where to find the header and library files of the dependencies for the build to succeed.
+If `pkg-config` is available (e.g. `apt-get install pkg-config` on Debian-based systems), it will
+automatically be used to locate the libraries and configure the correct build flags.
 Check your distribution's documentation to determine the location of the header and library files
@@ -156,3 +158,3 @@ or for more detailed information. When building on Alpine Linux (or any other distribution that
 ```shell
-usage: pystack remote [-h] [-v] [--no-color] [--no-block] [--native] [--native-all] [--locals] [--exhaustive] [--self] pid
+usage: pystack remote [-h] [-v] [--no-color] [--no-block] [--native] [--native-all] [--locals] [--exhaustive] pid
 
@@ -159,0 +161,0 @@ positional arguments:

src/pystack.egg-info/SOURCES.txt

@@ -10,2 +10,3 @@ .bumpversion.cfg
 setup.py
+build_scripts/elfutils_contiguous_segments.diff
 src/pystack/__init__.py
@@ -12,0 +13,0 @@ src/pystack/__main__.py

src/pystack/__main__.py

@@ -175,8 +175,2 @@ import argparse
     )
-    remote_parser.add_argument(
-        "--self",
-        action="store_true",
-        default=False,
-        help="Introspect the same process that invoke this program",
-    )
     core_parser = subparsers.add_parser(
@@ -183,0 +177,0 @@ "core",

src/pystack/_pystack.pyx

@@ -27,3 +27,2 @@ import contextlib
 from _pystack.mem cimport AbstractRemoteMemoryManager
-from _pystack.mem cimport BlockingProcessMemoryManager
 from _pystack.mem cimport MemoryMapInformation as CppMemoryMapInformation
@@ -36,2 +35,3 @@ from _pystack.mem cimport ProcessMemoryManager
 from _pystack.process cimport ProcessManager as NativeProcessManager
+from _pystack.process cimport ProcessTracer
 from _pystack.process cimport remote_addr_t
@@ -92,5 +92,5 @@ from _pystack.pycode cimport CodeObject
 
-cdef api void log_with_python(const char* message, int level):
-    with contextlib.suppress(UnicodeDecodeError):
-        LOGGER.log(level, message)
+cdef api void log_with_python(const cppstring *message, int level) noexcept:
+    pymessage = _try_to_decode_string(message)
+    LOGGER.log(level, pymessage)
 
@@ -295,2 +295,6 @@ T = TypeVar("T", bound=Callable[..., Any])
     def create_from_pid(cls, int pid, bint stop_process):
+        cdef shared_ptr[ProcessTracer] tracer
+        if stop_process:
+            tracer = make_shared[ProcessTracer](pid)
+
         virtual_maps = list(generate_maps_for_process(pid))
@@ -304,3 +308,3 @@ map_info = parse_maps_file(pid, virtual_maps)
             make_shared[NativeProcessManager](
-                pid, stop_process, analyzer,
+                pid, tracer, analyzer,
                 _pymaps_to_maps(virtual_maps),
@@ -662,4 +666,2 @@ _pymapinfo_to_mapinfo(map_info),
     )
-    virtual_maps = list(generate_maps_for_process(pid))
-    map_info = parse_maps_file(pid, virtual_maps)
 
@@ -666,0 +668,0 @@ try:

src/pystack/_pystack/logging.cpp

@@ -25,3 +25,3 @@ #include <stdexcept>
     if (!PyErr_Occurred()) {
-        log_with_python(message.c_str(), level);
+        log_with_python(&message, level);
     }
@@ -28,0 +28,0 @@ }

src/pystack/_pystack/mem.cpp

@@ -7,5 +7,3 @@ #include <algorithm>
 #include <memory>
-#include <sys/ptrace.h>
 #include <sys/uio.h>
-#include <sys/wait.h>
 #include <syscall.h>
@@ -295,47 +293,2 @@ #include <system_error>
 
-BlockingProcessMemoryManager::BlockingProcessMemoryManager(
-        pid_t pid,
-        const std::vector<int>& tids,
-        const std::vector<VirtualMap>& vmaps)
-: ProcessMemoryManager(pid, vmaps)
-, d_tids(tids)
-{
-    for (auto& tid : d_tids) {
-        LOG(INFO) << "Trying to stop thread " << tid;
-        long ret = ptrace(PTRACE_ATTACH, tid, nullptr, nullptr);
-        if (ret < 0) {
-            int error = errno;
-            detachFromProcess();
-            if (error == EPERM) {
-                throw std::runtime_error(PERM_MESSAGE);
-            }
-            throw std::system_error(error, std::generic_category());
-        }
-        LOG(INFO) << "Waiting for thread " << tid << " to be stopped";
-        ret = waitpid(tid, nullptr, WUNTRACED);
-        if (ret < 0) {
-            // In some old kernels is not possible to use WUNTRACED with
-            // threads (only the main thread will return a non zero value).
-            if (tid == pid || errno != ECHILD) {
-                detachFromProcess();
-            }
-        }
-        LOG(INFO) << "Process " << tid << " attached";
-    }
-}
-
-void
-BlockingProcessMemoryManager::detachFromProcess()
-{
-    for (auto& tid : d_tids) {
-        LOG(INFO) << "Detaching from thread " << tid;
-        ptrace(PTRACE_DETACH, tid, nullptr, nullptr);
-    }
-}
-
-BlockingProcessMemoryManager::~BlockingProcessMemoryManager()
-{
-    detachFromProcess();
-}
-
 CorefileRemoteMemoryManager::CorefileRemoteMemoryManager(
@@ -342,0 +295,0 @@ std::shared_ptr<CoreFileAnalyzer> analyzer,

src/pystack/_pystack/mem.h

@@ -182,22 +182,2 @@ #pragma once
 
-class BlockingProcessMemoryManager : public ProcessMemoryManager
-{
-  public:
-    // Constructors
-    explicit BlockingProcessMemoryManager(
-            pid_t pid,
-            const std::vector<int>& tids,
-            const std::vector<VirtualMap>& vmaps);
-
-    // Destructors
-    ~BlockingProcessMemoryManager() override;
-
-  private:
-    // Data members
-    std::vector<int> d_tids;
-
-    // Methods
-    void detachFromProcess();
-};
-
 struct SimpleVirtualMap
@@ -204,0 +184,0 @@ {

src/pystack/_pystack/mem.pxd

@@ -17,7 +17,2 @@ from libc.stdint cimport uintptr_t
 
-    cdef cppclass BlockingProcessMemoryManager(ProcessMemoryManager):
-        BlockingProcessMemoryManager(int pid, vector[int]d_tids) except+
-        ssize_t copyMemoryFromProcess(remote_addr_t addr, size_t size, void *destination) except+
-
-
     struct SimpleVirtualMap:
@@ -24,0 +19,0 @@ uintptr_t start

src/pystack/_pystack/process.cpp

@@ -9,2 +9,4 @@ #include <algorithm>
 #include <string>
+#include <sys/ptrace.h>
+#include <sys/wait.h>
 #include <utility>
@@ -27,2 +29,4 @@ #include <vector>
 
+static const std::string PERM_MESSAGE = "Operation not permitted";
+
 class DirectoryReader
@@ -79,2 +83,79 @@ {
 
+ProcessTracer::ProcessTracer(pid_t pid)
+{
+    std::unordered_map<int, int> error_by_tid;
+
+    bool found_new_tid = true;
+    while (found_new_tid) {
+        found_new_tid = false;
+
+        auto tids = getProcessTids(pid);
+        for (auto& tid : tids) {
+            if (d_tids.count(tid)) {
+                continue;  // already stopped
+            }
+
+            auto err_it = error_by_tid.find(tid);
+            if (err_it != error_by_tid.end()) {
+                // We got an error for this TID on the last iteration.
+                // Since we found the TID again this iteration, it still
+                // belongs to us and should have been stoppable.
+                detachFromProcess();
+
+                int error = err_it->second;
+                if (error == EPERM) {
+                    throw std::runtime_error(PERM_MESSAGE);
+                }
+                throw std::system_error(error, std::generic_category());
+            }
+
+            found_new_tid = true;
+
+            LOG(INFO) << "Trying to stop thread " << tid;
+            long ret = ptrace(PTRACE_ATTACH, tid, nullptr, nullptr);
+            if (ret < 0) {
+                int error = errno;
+                LOG(WARNING) << "Failed to attach to thread " << tid << ": " << strerror(error);
+                error_by_tid.emplace(tid, error);
+                continue;
+            }
+
+            // Add each tid as we attach: these are the tids we detach from.
+            d_tids.insert(tid);
+
+            LOG(INFO) << "Waiting for thread " << tid << " to be stopped";
+            ret = waitpid(tid, nullptr, WUNTRACED);
+            if (ret < 0) {
+                // In some old kernels is not possible to use WUNTRACED with
+                // threads (only the main thread will return a non zero value).
+                if (tid == pid || errno != ECHILD) {
+                    detachFromProcess();
+                }
+            }
+            LOG(INFO) << "Thread " << tid << " stopped";
+        }
+    }
+    LOG(INFO) << "All " << d_tids.size() << " threads stopped";
+}
+
+void
+ProcessTracer::detachFromProcess()
+{
+    for (auto& tid : d_tids) {
+        LOG(INFO) << "Detaching from thread " << tid;
+        ptrace(PTRACE_DETACH, tid, nullptr, nullptr);
+    }
+}
+
+ProcessTracer::~ProcessTracer()
+{
+    detachFromProcess();
+}
+
+std::vector<int>
+ProcessTracer::getTids() const
+{
+    return {d_tids.begin(), d_tids.end()};
+}
+
 AbstractProcessManager::AbstractProcessManager(
@@ -545,3 +626,3 @@ pid_t pid,
         pid_t pid,
-        bool blocking,
+        const std::shared_ptr<ProcessTracer>& tracer,
         const std::shared_ptr<ProcessAnalyzer>& analyzer,
@@ -551,9 +632,10 @@ std::vector<VirtualMap> memory_maps,
 : AbstractProcessManager(pid, std::move(memory_maps), std::move(map_info))
-, d_tids(getProcessTids(pid))
+, tracer(tracer)
 {
-    if (blocking) {
-        d_manager = std::make_unique<BlockingProcessMemoryManager>(pid, d_tids, d_memory_maps);
+    if (tracer) {
+        d_tids = tracer->getTids();
     } else {
-        d_manager = std::make_unique<ProcessMemoryManager>(pid, d_memory_maps);
+        d_tids = getProcessTids(pid);
     }
+    d_manager = std::make_unique<ProcessMemoryManager>(pid, d_memory_maps);
     d_analyzer = analyzer;
@@ -560,0 +642,0 @@ d_unwinder = std::make_unique<Unwinder>(analyzer);

src/pystack/_pystack/process.h

@@ -10,2 +10,3 @@ #pragma once
 #include <unordered_map>
+#include <unordered_set>
 #include <utility>
@@ -31,2 +32,24 @@ #include <vector>
 
+class ProcessTracer
+{
+  public:
+    // Constructors
+    ProcessTracer(pid_t pid);
+    ProcessTracer(const ProcessTracer&) = delete;
+    ProcessTracer& operator=(const ProcessTracer&) = delete;
+
+    // Destructors
+    ~ProcessTracer();
+
+    // Methods
+    std::vector<int> getTids() const;
+
+  private:
+    // Data members
+    std::unordered_set<int> d_tids;
+
+    // Methods
+    void detachFromProcess();
+};
+
 class AbstractProcessManager : public std::enable_shared_from_this<AbstractProcessManager>
@@ -123,3 +146,3 @@ {
             pid_t pid,
-            bool blocking,
+            const std::shared_ptr<ProcessTracer>& tracer,
             const std::shared_ptr<ProcessAnalyzer>& analyzer,
@@ -134,2 +157,3 @@ std::vector<VirtualMap> memory_maps,
     // Data members
+    std::shared_ptr<ProcessTracer> tracer;
     std::vector<int> d_tids;
@@ -136,0 +160,0 @@ };

src/pystack/_pystack/process.pxd

@@ -20,2 +20,5 @@ from _pystack.elf_common cimport CoreFileAnalyzer
 cdef extern from "process.h" namespace "pystack":
+    cdef cppclass ProcessTracer:
+        pass
+
     cdef cppclass AbstractProcessManager:
@@ -34,5 +37,5 @@ remote_addr_t scanBSS() except+
     cdef cppclass ProcessManager(AbstractProcessManager):
-        ProcessManager(int pid, int blocking, shared_ptr[ProcessAnalyzer] analyzer, vector[VirtualMap] memory_maps, MemoryMapInformation map_info) except+
+        ProcessManager(int pid, shared_ptr[ProcessTracer] tracer, shared_ptr[ProcessAnalyzer] analyzer, vector[VirtualMap] memory_maps, MemoryMapInformation map_info) except+
 
     cdef cppclass CoreFileProcessManager(AbstractProcessManager):
         CoreFileProcessManager(int pid, shared_ptr[CoreFileAnalyzer] analyzer, vector[VirtualMap] memory_maps, MemoryMapInformation map_info) except+

src/pystack/_pystack/pythread.cpp

@@ -99,5 +99,16 @@ #include <algorithm>
         // Attempt to locate a field in the pthread struct that's equal to the pid.
-        uintptr_t buffer[200];
-        manager->copyObjectFromProcess(pthread_id_addr, &buffer);
-        for (int i = 0; i < 200; i++) {
+        uintptr_t buffer[100];
+        size_t buffer_size = sizeof(buffer);
+        while (buffer_size > 0) {
+            try {
+                LOG(DEBUG) << "Trying to copy a buffer of " << buffer_size << " bytes to get pthread ID";
+                manager->copyMemoryFromProcess(pthread_id_addr, buffer_size, &buffer);
+                break;
+            } catch (const RemoteMemCopyError& ex) {
+                LOG(DEBUG) << "Failed to copy buffer to get pthread ID";
+                buffer_size /= 2;
+            }
+        }
+        LOG(DEBUG) << "Copied a buffer of " << buffer_size << " bytes to get pthread ID";
+        for (size_t i = 0; i < buffer_size / sizeof(uintptr_t); i++) {
             if (static_cast<pid_t>(buffer[i]) == manager->Pid()) {
@@ -104,0 +115,0 @@ off_t offset = sizeof(uintptr_t) * i;

src/pystack/_version.py

@@ -1,1 +0,1 @@
-__version__ = "1.2.0"
+__version__ = "1.3.0"

Alert delta unavailable

Currently unable to show alert delta for PyPI packages.

Improved metrics

Total package byte prevSize

375119

1.46%

Number of package files

80

1.27%

Lines of code

1349

0.22%