Detect GIL on 3.9.3, 3.9.4, 3.8.9

[Jongy]

Tested py-spy record --gil on the following Docker images: python:3.9.3, python:3.9.4, python:3.8.9 - works now, hasn't worked before.

[benfred]

thanks!

[benfred]

Fwiw - I'm trying to start doing a better job of testing compatibility across python versions here #378 . The idea is after the build stage we will take the built python wheel and install it with all the different versions of python that github actions supports (~35 right now) , and run some basic tests of the CLI app that gets installed.

This should catch GIL issues for the versions that we're targeting here - and the next step will be to automatically update the tested python versions as newer versions get released. I've created a script to update the python_versions in the github actions workflow in this PR, but want to set this up to run nightly and submit a PR if there is any changes.

Once we're automatically testing out all python versions, I'd like to either change to either also automatically generating the changes for GIL offsets, or just assume they don't change between patch releases in the code and rely on the tests to highlight failures if this isn't the case.

[benfred]

I've added support to automatically expand the set of tested python versions - it should run nightly now with this workflow https://github.com/benfred/py-spy/blob/master/.github/workflows/update_python_test.yml and create PR's that will look like #382 when there is a new python version available to test

[Jongy]

I've added support to automatically expand the set of tested python versions - it should run nightly now with this workflow https://github.com/benfred/py-spy/blob/master/.github/workflows/update_python_test.yml and create PR's that will look like #382 when there is a new python version available to test

Well, that's cool :)

Adding another GH action that runs the offsets scripts and creates a PR with it doesn't sound too complex.

Also, I think that for the GIL case, since it's very unlikely to succeed if you give it bad offsets (it's comparing pointers of PyThreadStates if I recall?) - then just assuming the offsets remain the same, committing it and trying might work for most versions I guess. And we can expect it to fail hard if the offsets are indeed wrong (as in - fail to get any stacks, which we can easily check).

[benfred]

then just assuming the offsets remain the same, committing it and trying might work for most versions I guess.

Yeah, the test script that is running against each python version is pretty minimal - but should detect issues if the GIL offsets are wrong (one test sets the --gil flag and expects samples, and another sets the --gil flag and expects to not get samples). I'm also planning to expand the set of tests over time by incorporating more logic from the rust integration tests.

I'm thinking that once we're confident that the test coverage will automatically test new versions of python- we can just assume the GIL offsets are the same for each patch release of python, and fix if this isn't the case. The only real time that the GIL offset changed between patch releases was going from v3.7.3 to 3.7.4 , so this seems like a reasonable assumption for the most part. The advantage with this is that we don't need to release a new version of py-spy with every python patch - and will only need to do releases when the GIL offset changes (which is relatively infrequently).

I believe v3.9.5 is scheduled for release next week, and will keep an eye out to see if this works with a real new version of python =)

[Jongy]

Sounds good.

I believe v3.9.5 is scheduled for release next week, and will keep an eye out to see if this works with a real new version of python =)

Let's watch :) I'll keep an eye for that.

[benfred]

We got a PR for testing out 3.9.5 / 3.8.10 here #384 .

It didn't fail it immediately (like the checks for those versions are still going on right now) - but I still think this is a win for automatically testing new versions.

The reason the tests are hanging right now is this code

py-spy/src/python_spy.rs

Lines 49 to 53 in 0a1f4ca

	if config.gil_only {
	eprintln!("Cannot detect GIL holding in version '{}' on the current platform (reason: {})", version, msg);
	eprintln!("Please open an issue in https://github.com/benfred/py-spy with the Python version and your platform.");
	std::process::exit(1);
	}

. The error message is printing out, and the py-spy process exits - but this doesn't clean up or kill the profiled python subprocess that py-spy launches. The test is using check_output

py-spy/tests/integration_test.py

Line 41 in c77c543

subprocess.check_output(cmdline)

which waits for all the subprocesses (python script as well as py-spy process) , so it ends up running forever.

I'm going to change this to clean up the profiled process rather than using std::process::exit(1) here #385