Fix profiling on windows with python3.12 (#774)

* Fix profiling on windows with python3.12

Windows was failing on python 3.12 with an error message like

```
Only part of a ReadProcessMemory or WriteProcessMemory request was completed. (os error 299)
```

This was caused by incorrect handling of getting the gil_thread_id.

* temporarily pause clippy linting

* Migrate to upload-artifact@v4

* .

* test on ubuntu 24.04 instead of 20.04

* 22.04

* Update python test versions

Author: benfred

.github/workflows/build.yml

@@ -76,12 +76,18 @@ jobs:
         run: |
           python3 -c "import shutil; import glob; wheels = glob.glob('dist/*.whl'); [shutil.move(wheel, wheel.replace('py3', 'py2.py3')) for wheel in wheels if 'py2' not in wheel]"
         if: runner.os != 'Linux'
-      - name: Upload wheels
-        uses: actions/upload-artifact@v3
+      - name: Upload Windows Wheels
+        uses: actions/upload-artifact@v4
         with:
-          name: wheels
+          name: wheels-windows
           path: dist
-        if: runner.os != 'Linux'
+        if: runner.os == 'Windows'
+      - name: Upload Macos Wheels
+        uses: actions/upload-artifact@v4
+        with:
+          name: wheels-macos
+          path: dist
+        if: runner.os == 'macOS'
 
   build-linux-cross:
     runs-on: ubuntu-latest
@@ -120,9 +126,9 @@ jobs:
         run: |
           python3 -c "import shutil; import glob; wheels = glob.glob('dist/*.whl'); [shutil.move(wheel, wheel.replace('py3', 'py2.py3')) for wheel in wheels if 'py2' not in wheel]"
       - name: Upload wheels
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
-          name: wheels
+          name: wheels-${{ matrix.target }}
           path: dist
 
   build-freebsd:
@@ -135,20 +141,6 @@ jobs:
           - freebsd-14
     steps:
       - uses: actions/checkout@v3
-      - name: Cache Vagrant box
-        uses: actions/cache@v3.0.4
-        with:
-          path: .vagrant.d
-          key: ${{ matrix.box }}-vagrant-boxes-20231115-${{ hashFiles('ci/Vagrantfile') }}
-          restore-keys: |
-            ${{ matrix.box }}-vagrant-boxes-20231115-
-      - name: Cache Cargo and build artifacts
-        uses: actions/cache@v3.0.4
-        with:
-          path: build-artifacts.tar
-          key: ${{ matrix.box }}-cargo-20231115-${{ hashFiles('**/Cargo.lock') }}
-          restore-keys: |
-            ${{ matrix.box }}-cargo-20231115-
       - name: Display CPU info
         run: lscpu
       - name: Install VM tools
@@ -177,7 +169,7 @@ jobs:
           tar xf build-artifacts.tar target/release/py-spy
           mv target/release/py-spy py-spy-x86_64-unknown-freebsd
       - name: Upload Binaries
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: py-spy-x86_64-unknown-freebsd
           path: py-spy-x86_64-unknown-freebsd
@@ -199,11 +191,11 @@ jobs:
             3.8.0,
             3.8.18,
             3.9.0,
-            3.9.20,
+            3.9.23,
             3.10.0,
-            3.10.15,
+            3.10.18,
             3.11.0,
-            3.11.10,
+            3.11.13,
             3.12.0,
             3.12.1,
             3.12.2,
@@ -212,64 +204,79 @@ jobs:
             3.12.5,
             3.12.6,
             3.12.7,
+            3.12.8,
+            3.12.9,
+            3.12.10,
+            3.12.11,
             3.13.0,
+            3.13.1,
+            3.13.2,
+            3.13.3,
+            3.13.4,
+            3.13.5,
           ]
-        # TODO: also test windows
-        os: [ubuntu-20.04, macos-13, windows-latest]
+        os: [ubuntu-22.04, macos-13, windows-latest]
         # some versions of python can't be tested on GHA with osx because of SIP:
         exclude:
+          - os: ubuntu-22.04
+            python-version: 3.6.7
           - os: windows-latest
             python-version: 3.6.15
+          - os: ubuntu-22.04
+            python-version: 3.6.15
+          - os: ubuntu-22.04
+            python-version: 3.7.1
           - os: windows-latest
             python-version: 3.7.17
+          - os: ubuntu-22.04
+            python-version: 3.8.0
           - os: windows-latest
             python-version: 3.8.18
+          - os: ubuntu-22.04
+            python-version: 3.9.0
           - os: windows-latest
-            python-version: 3.9.20
+            python-version: 3.9.23
+          - os: ubuntu-22.04
+            python-version: 3.10.0
           - os: windows-latest
-            python-version: 3.10.15
+            python-version: 3.10.18
           - os: macos-13
-            python-version: 3.11.10
+            python-version: 3.11.13
           - os: windows-latest
-            python-version: 3.11.10
+            python-version: 3.11.13
           - os: macos-13
             python-version: 3.12.0
-          - os: windows-latest
-            python-version: 3.12.0
           - os: macos-13
             python-version: 3.12.1
-          - os: windows-latest
-            python-version: 3.12.1
           - os: macos-13
             python-version: 3.12.2
-          - os: windows-latest
-            python-version: 3.12.2
           - os: macos-13
             python-version: 3.12.3
-          - os: windows-latest
-            python-version: 3.12.3
           - os: macos-13
             python-version: 3.12.4
-          - os: windows-latest
-            python-version: 3.12.4
           - os: macos-13
             python-version: 3.12.5
-          - os: windows-latest
-            python-version: 3.12.5
           - os: macos-13
             python-version: 3.12.6
-          - os: windows-latest
-            python-version: 3.12.6
           - os: macos-13
             python-version: 3.12.7
+          - os: macos-13
+            python-version: 3.12.8
+          - os: macos-13
+            python-version: 3.12.9
+          - os: macos-13
+            python-version: 3.12.10
+          - os: macos-13
+            python-version: 3.12.11
           - os: windows-latest
-            python-version: 3.12.7
+            python-version: 3.12.11
 
     steps:
       - uses: actions/checkout@v2
-      - uses: actions/download-artifact@v3
+      - uses: actions/download-artifact@v4
         with:
-          name: wheels
+          pattern: wheels-*
+          merge-multiple: true
       - uses: actions/setup-python@v4
         with:
           python-version: ${{ matrix.python-version }}
@@ -311,9 +318,10 @@ jobs:
     if: "startsWith(github.ref, 'refs/tags/')"
     needs: [test-wheels]
     steps:
-      - uses: actions/download-artifact@v3
+      - uses: actions/download-artifact@v4
         with:
-          name: wheels
+          pattern: wheels-*
+          merge-multiple: true
       - name: Create GitHub Release
         uses: fnkr/github-action-ghr@v1.3
         env:

.pre-commit-config.yaml

@@ -11,14 +11,6 @@ repos:
     hooks:
       - id: fmt
       - id: cargo-check
-  - repo: local
-    hooks:
-      - id: cargo-clippy
-        name: cargo clippy
-        entry: cargo clippy -- -D warnings
-        language: system
-        files: \.rs$
-        pass_filenames: false
   - repo: https://github.com/rbubley/mirrors-prettier
     rev: v3.3.3
     hooks:

ci/update_python_test_versions.py

@@ -19,7 +19,13 @@ def get_github_python_versions():
     # get a map of version: platform/arch so we can exclude here
     platforms = {}
     for v in versions_json:
-        platforms[v["version"]] = set((f["platform"], f["arch"]) for f in v["files"])
+        version_platforms = set()
+        for f in v["files"]:
+            platform, arch = f["platform"], f["arch"]
+            if platform == "linux" and f.get("platform_version") != "22.04":
+                continue
+            version_platforms.add((platform, arch))
+        platforms[v["version"]] = version_platforms
 
     raw_versions = [v["version"] for v in versions_json]
     minor_versions = defaultdict(list)
@@ -63,7 +69,7 @@ def update_python_test_versions():
         pathlib.Path(__file__).parent.parent / ".github" / "workflows" / "build.yml"
     )
 
-    build_yml = yaml.safe_load(open(".github/workflows/build.yml"))
+    build_yml = yaml.safe_load(open(build_yml_path))
     test_matrix = build_yml["jobs"]["test-wheels"]["strategy"]["matrix"]
     existing_python_versions = test_matrix["python-version"]
     if versions == existing_python_versions:
@@ -95,10 +101,14 @@ def update_python_test_versions():
             exclusions.append("          - os: macos-13\n")
             exclusions.append(f"            python-version: {v}\n")
 
-        if ("win32", "x64") not in platforms[v] or v.startswith("3.12"):
+        if ("win32", "x64") not in platforms[v]:
             exclusions.append("          - os: windows-latest\n")
             exclusions.append(f"            python-version: {v}\n")
 
+        if ("linux", "x64") not in platforms[v]:
+            exclusions.append("          - os: ubuntu-22.04\n")
+            exclusions.append(f"            python-version: {v}\n")
+
     first_exclude_line = lines.index("        exclude:\n", first_line)
     last_exclude_line = lines.index("\n", first_exclude_line)
     lines = lines[: first_exclude_line + 1] + exclusions + lines[last_exclude_line:]

src/coredump.rs

@@ -247,7 +247,7 @@ impl PythonCoreDump {
         // lets us figure out which thread has the GIL
         let config = Config::default();
         let threadstate_address =
-            get_threadstate_address(interpreter_address, &python_info, &version, &config)?;
+            get_threadstate_address(interpreter_address, &python_info, &core, &version, &config)?;
         info!("found threadstate at 0x{:016x}", threadstate_address);
 
         Ok(PythonCoreDump {

src/python_interpreters.rs

@@ -22,8 +22,9 @@ pub trait InterpreterState {
     type StringObject: StringObject;
     type ListObject: ListObject;
     type TupleObject: TupleObject;
+    const HAS_GIL_RUNTIME_STATE: bool = false;
+
     fn head(&self) -> *mut Self::ThreadState;
-    fn gil_locked(&self) -> Option<bool>;
     fn modules(&self) -> *mut Self::Object;
 }
 
@@ -117,9 +118,6 @@ macro_rules! PythonCommonImpl {
             fn head(&self) -> *mut Self::ThreadState {
                 self.tstate_head
             }
-            fn gil_locked(&self) -> Option<bool> {
-                None
-            }
             fn modules(&self) -> *mut Self::Object {
                 self.modules
             }
@@ -416,12 +414,11 @@ impl InterpreterState for v3_13_0::PyInterpreterState {
     type StringObject = v3_13_0::PyUnicodeObject;
     type ListObject = v3_13_0::PyListObject;
     type TupleObject = v3_13_0::PyTupleObject;
+    const HAS_GIL_RUNTIME_STATE: bool = true;
+
     fn head(&self) -> *mut Self::ThreadState {
         self.threads.head
     }
-    fn gil_locked(&self) -> Option<bool> {
-        Some(self._gil.locked != 0)
-    }
     fn modules(&self) -> *mut Self::Object {
         self.imports.modules
     }
@@ -500,14 +497,11 @@ impl InterpreterState for v3_12_0::PyInterpreterState {
     type StringObject = v3_12_0::PyUnicodeObject;
     type ListObject = v3_12_0::PyListObject;
     type TupleObject = v3_12_0::PyTupleObject;
+    const HAS_GIL_RUNTIME_STATE: bool = true;
 
     fn head(&self) -> *mut Self::ThreadState {
         self.threads.head
     }
-    fn gil_locked(&self) -> Option<bool> {
-        Some(self._gil.locked._value != 0)
-    }
-
     fn modules(&self) -> *mut Self::Object {
         self.imports.modules
     }
@@ -593,12 +587,10 @@ impl InterpreterState for v3_11_0::PyInterpreterState {
     type StringObject = v3_11_0::PyUnicodeObject;
     type ListObject = v3_11_0::PyListObject;
     type TupleObject = v3_11_0::PyTupleObject;
+
     fn head(&self) -> *mut Self::ThreadState {
         self.threads.head
     }
-    fn gil_locked(&self) -> Option<bool> {
-        None
-    }
     fn modules(&self) -> *mut Self::Object {
         self.modules
     }

src/python_process_info.rs

@@ -588,30 +588,32 @@ where
     }
 }
 
-pub fn get_threadstate_address(
+pub fn get_threadstate_address<P>(
     interpreter_address: usize,
     python_info: &PythonProcessInfo,
+    process: &P,
     version: &Version,
     config: &Config,
-) -> Result<usize, Error> {
+) -> Result<usize, Error>
+where
+    P: ProcessMemory,
+{
     let threadstate_address = match version {
         Version {
             major: 3,
             minor: 13,
             ..
         } => {
-            let interp: v3_13_0::_is = Default::default();
-            let offset = crate::utils::offset_of(&interp, &interp._gil.last_holder);
-            interpreter_address + offset
+            let interp: v3_13_0::_is = process.copy_struct(interpreter_address)?;
+            interp.ceval.gil as usize
         }
         Version {
             major: 3,
             minor: 12,
             ..
         } => {
-            let interp: v3_12_0::_is = Default::default();
-            let offset = crate::utils::offset_of(&interp, &interp._gil.last_holder._value);
-            interpreter_address + offset
+            let interp: v3_12_0::_is = process.copy_struct(interpreter_address)?;
+            interp.ceval.gil as usize
         }
         Version {
             major: 3,

src/python_spy.rs

@@ -62,8 +62,13 @@ impl PythonSpy {
         info!("Found interpreter at 0x{:016x}", interpreter_address);
 
         // lets us figure out which thread has the GIL
-        let threadstate_address =
-            get_threadstate_address(interpreter_address, &python_info, &version, config)?;
+        let threadstate_address = get_threadstate_address(
+            interpreter_address,
+            &python_info,
+            &process,
+            &version,
+            config,
+        )?;
 
         #[cfg(feature = "unwind")]
         let native = if config.native {
@@ -219,11 +224,8 @@ impl PythonSpy {
             .context("Failed to copy PyInterpreterState from process")?;
 
         // get the threadid of the gil if appropriate
-        let gil_thread_id = if interp.gil_locked().unwrap_or(true) {
-            get_gil_threadid::<I, Process>(self.threadstate_address, &self.process)?
-        } else {
-            0
-        };
+        let gil_thread_id = get_gil_threadid::<I, Process>(self.threadstate_address, &self.process)
+            .context("failed to get gil_thread_id")?;
 
         let mut traces = Vec::new();
         let mut threads = interp.head();