[PATCH] D62371: [llvm-symbolizer] Flush output on bad input

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin added inline comments.


================
Comment at: test/tools/llvm-symbolizer/Inputs/flush-output.py:8
+    process.kill()
+    sys.exit(1)
+
----------------
jhenderson wrote:
> ikudrin wrote:
> > It looks like when `process.kill()` is executed, the main thread stops waiting, processes the rest and exits with the exit status `0`.
> > 
> > [[ https://docs.python.org/3/library/sys.html#sys.exit | The python documentation]] says:
> > > Since exit() ultimately “only” raises an exception, it will only exit the process when called from the main thread, and the exception is not intercepted.
> I'm not sure what you're asking me to do here? This code causes llvm-symbolizer to end, rather than continuing to hang, and then the python script exits.
> 
> For reference, I verified that the test fails and only hangs for 20 seconds as expected, without the bug fix, and passes with the bug fix.
Yes, the test fails, but not because the python script finishes with the exit code "1". The actual failure is that FileCheck cannot find all "CHECK" lines. Thus, the line `sys.exit(1)` is a bit of misleading.

If I understand your intention right, I would suggest rewriting the script in the following way:
```
exit_status = 0

def kill_subprocess(process):
    global exit_status
    exit_status = 1
    process.kill()

cmd = ...
...
sys.exit(exit_status)
```


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62371/new/

https://reviews.llvm.org/D62371