kcov --python-parser=python3 hangs if python3 is not installed

[asomers]

Running kcov --python-parser=python3 hangs if python3 is not installed. The worst part is that SIGTERM is insufficient to kill kcov, only SIGKILL will work. The hang is caused by the parent attempting to open a named pipe in PythonEngine::start on this line:

kcov/src/engines/python-engine.cc

Line 110 in 88e2804

m_pipe = fopen(kcov_python_pipe_path.c_str(), "r");

However, the child has already exited 6 lines above. Since the child never opened its end of the named pipe, fopen never returns.

One way to fix this bug would be to use pipe(2) to open an already-connected pair of pipes instead of mkfifo. Then one end of the opened pipe could be passed to the child. When the child dies, the other end of the pipe would get EPIPE instead of hanging.

I've reproduced this issue on Debian 9.3 and FreeBSD 11.1. The easiest way to reproduce it, on a system that may or may not have python3 installed, is to simply provide a bogus name for the python interpreter, like this:
kcov /tmp/kcov --python-parser=python4 tests/python/main 5

[SimonKagstrom]

Thanks for the detailed analysis!

Pull-request coming up? I'll fix it myself otherwise.

I guess it would also be good to check if the command actually exists first, and bail out if it doesn't.

[asomers]

I'm not preparing a PR for this right now. I'm focusing on getting ptrace support to work on FreeBSD.

[SimonKagstrom]

OK, I'll fix it myself!

Pleased to hear that you're working on FreeBSD support! I've thought about it myself, but don't run any of the BSDs. ptrace.cc is probably full of linuxisms, intentional or not.

[SimonKagstrom]

Fixed with 2651b54, at least partially.

The fix is only partial in the sense that it checks if the python parser exists in the PATH (or via an absolute path). It could still hang if something bad but existing is passed to the command.

The other end of the pipe is in a Python script (python-helper.py), so a named pipe felt as an easier implementation.

Leaving open for now, although it should not be as bad as before.

[asomers]

Thanks!

[jack-rann]

Hi @SimonKagstrom, for the work on issue #458 I looked at how both the python and bash engines invoke the script under test. python-engine.cc uses system() which suffers from the potential hang reported above. I'm proposing changing bash-engine.cc to use execvp(). A simple test shows that this approach doesn't seem to suffer from the same hang.

This approach would also make argument handling in python-engine.cc more robust. As it stands now, the code simply wraps all command line arguments in single quotes before invoking system(). This leads to problems if any argument contains a single quote. For example:

$ kcov --python-parser=python3 coverage src/app.py 1 2 "a'b"
sh: -c: line 0: unexpected EOF while looking for matching `''
sh: -c: line 1: syntax error: unexpected end of file
^Ckcov: error: Can't open python pipe coverage//app.py.4bb25db1742795a6/kcov-python.pipe
$ 

Note that kcov hangs with the invocation above and it is killed via ^C.

Using execvp() in place of system() appears to work nicely:

$ ../build/src/kcov --python-parser=python3 coverage src/app.py 1 2 "a'b"
args: ['src/app.py', '1', '2', "a'b"]
in func1
in func2
$ 

If this is a reasonable approach, I'd be happy to work a PR here.

[SimonKagstrom]

@jack-rann sounds like a great idea!

Thanks for the findings and explanation of them, and a PR would be great!

[SimonKagstrom]

@jack-rann I suppose this should be closed now as well, after your PR #466 ?

[jack-rann]

Not yet, @SimonKagstrom. PR #466 only affected Bash. I’ll start on this Python issue once I return next week.

[jack-rann]

Will plan to implement the same argument processing and execvp() handling done in PR #466 in python-engine.cc.

Existing Python tests should be adequate for the execvp() modifications. Created a new tests/python/echo.py script for testing of correct argument handling.

Manually invoking unmodified kcov with an argument containing a single quote yields the same hang as reported above:

$ src/kcov --python-parser=python3 c ../tests/python/echo.py 1 two "a'b"
sh: -c: line 0: unexpected EOF while looking for matching `''
sh: -c: line 1: syntax error: unexpected end of file
^Ckcov: error: Can't open python pipe c//echo.py.adad0d08d907c715/kcov-python.pipe
$ 

Execution of the new test_python.python_single_quote_arg.runTest will similarly hang.

[jack-rann]

After code modification, python-engine.cc handles sub-command execution in the same manner as bash-engine.cc.

Manual execution of test script above now succeeds. Note that the embedded single quote character in an argument is now properly handled:

$ src/kcov --python-parser=python3 c ../tests/python/echo.py 1 two "a'b"
echo: ['../tests/python/echo.py', '1', 'two', "a'b"]
$ 

Local execution of Python related tests succeed:

...
runTest (test_python.python2_can_set_legal_parser.runTest) ... skipped 'python2 not available'
runTest (test_python.python2_coverage.runTest) ... skipped 'python2 not available'
runTest (test_python.python3_coverage.runTest) ... ok
runTest (test_python.python_accumulate_data.runTest) ... ok
runTest (test_python.python_can_set_illegal_parser.runTest) ... ok
runTest (test_python.python_can_set_legal_parser.runTest) ... ok
runTest (test_python.python_coverage.runTest) ... ok
runTest (test_python.python_exit_status.runTest) ... ok
runTest (test_python.python_issue_368_can_handle_symlink_target.runTest) ... ok
runTest (test_python.python_single_quote_arg.runTest) ... ok
runTest (test_python.python_tricky_single_dict_assignment.runTest) ... expected failure
runTest (test_python.python_tricky_single_line_string_assignment.runTest) ... ok
runTest (test_python.python_unittest.runTest) ... ok
...

CI tests on GitHub pass for changes made on this branch. https://github.com/jack-rann/kcov/actions/runs/11087951720

Will open PR shortly.