subprocess Import Failure in tests/deadlock.py

[a-gavin]

In cloning/configuring the repository as instructed in Getting Started, I ran into the following error in the make check step (partial output from tests/test-suite.log):

FAIL: deadlock
==============

Accept pattern found: Waiting for input.
Accept pattern found: hello
Applying live patch.
ulp: reading live patch metadata from libblocked_livepatch1.ulp.
ulp: getting in-memory information about process 3179.
ulp: getting in-memory information about the main executable.
ulp: PTRACE_ATTACH error: Operation not permitted.

ulp: unable to attach to 3179 to read data.

ulp: error: unable to read PHDR entry
ulp: unable to calculate the load bias for the executable.
ulp: unable to get in-memory information about the main executable.
ulp: error gathering target process information.
Traceback (most recent call last):
  File "./deadlock.py", line 43, in <module>
    child.livepatch('libblocked_livepatch1.ulp', retries=100, timeout=20)
  File "/tmp/libpulp/tests/testsuite.py", line 204, in livepatch
    tool.check_returncode()
  File "/usr/lib/python3.8/subprocess.py", line 448, in check_returncode
    raise CalledProcessError(self.returncode, self.args, self.stdout,
subprocess.CalledProcessError: Command '['/tmp/libpulp/tests/../tools/ulp_trigger', '-p', '3179', 'libblocked_livepatch1.ulp', '-v', '-r', '100']' returned non-zero exit status 1.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "./deadlock.py", line 44, in <module>
    except subprocess.TimeoutExpired:
NameError: name 'subprocess' is not defined
FAIL deadlock.py (exit status: 1)

Issue is caused by C-style reliance on the import of subprocess in tests/testsuite.py on this line. Module subprocess is only visible within the scope of testsuite, not deadlock.py. See this StackOverflow post for a better explanation of the issue generally.

This issue can be fixed by directly importing subprocess in tests/testsuite.py or by changing the usage to reference the import within testsuite (i.e. except testsuite.subprocess.TimeoutExpired), depending on project preference. I have verified that both methods prevent the second exception from occurring.

[a-gavin]

To follow up on this, the bug occurred while attempting to setup the package to test/do development on in a Docker container. Most tests failed initially (thus allowing for this bug to occur).

Running the container with --cap-add=SYS_PTRACE resulted in all tests performing as expected (testsuite passed) when running make check within the container itself. A more thorough explanation can be found here.

[inconstante]

Issue is caused by C-style reliance on the import of subprocess in tests/testsuite.py on this line. Module subprocess is only visible within the scope of testsuite, not deadlock.py. See this StackOverflow post for a better explanation of the issue generally.

Thanks for the detailed analysis and explanation.

This issue can be fixed by directly importing subprocess in tests/testsuite.py or by changing the usage to reference the import within testsuite (i.e. except testsuite.subprocess.TimeoutExpired), depending on project preference. I have verified that both methods prevent the second exception from occurring.

There's no project-wide preference. If I were to choose, I'd probably import subprocess in deadlock.py to avoid making lines too long, but that's more personal taste than project guidelines. If you were to write a patch, you could do it according to your own preference.

Would you be willing to send a pull request with this fix?

[a-gavin]

Definitely! I agree on the long lines. PR submitted

[inconstante]

Fixed by #22