[lldb-mi] Fix hanging of target-select-so-path.test

[apolyakov]

Summary

The target-select-so-path test might hang on
some platforms. The reason of that behavior
was in incorrect usage of Filecheck and lldb-mi
processes. Instead of redirecting lldb-mi's output
to Filecheck, we should run lldb-mi session,
finish the session, collect its output and then pass
it to Filecheck.
Also, this patch adds a timer to the test to prevent
it from hanging in the future.

[aprantl]

It looks like it would be easy to forget to add -gdb-exit to every test. Wouldn't it be better to have the lldb-mi automatically produce a -gdb-exit command when it reads EOF from stdin?

[apolyakov]

It seems reasonable, I'll look at this.

[apolyakov]

I found out that the reason of hanging of the test suite was in incorrect usage of Filecheck and lldb-mi processes in target-select-so-path.test. Current patch has been tested on CentOS 7.

[tatyana-krasnukha]

These changes fixed the issue for me.

[teemperor]

FYI, for me the test is still hanging on Arch Linux.

[apolyakov]

Can you provide some logs? For example, you might be able to run this test (llvm-lit -a -vv .../target-select-so-path.test) and kill lldb-mi and filecheck processes if they hang. Also, information about which processes hang would be useful too (lldb-mi or filecheck or maybe both).

[tatyana-krasnukha]

Alexander, please add a timeout to be sure that the test is not hanging even if something goes wrong. A possible solution for Python 2:

target-select-so-path.py.patch1 KBDownload

[apolyakov]

Thanks to @tatyana-krasnukha for the idea about a timer. Added a timer to target-select-so-path test.

[teemperor]

I'm accepting this as seems to improve the situation, even though it's not a complete fix.

(Not sure about the 2-minute timeout value though, but we'll see how the bots react).

[apolyakov]

In D52139#1240835, @apolyakov wrote:

Can you provide some logs? For example, you might be able to run this test (llvm-lit -a -vv .../target-select-so-path.test) and kill lldb-mi and filecheck processes if they hang. Also, information about which processes hang would be useful too (lldb-mi or filecheck or maybe both).

@teemperor can you do this? It'll help me a lot.

[teemperor]

Sure, here is the output: After some debugging it seems that our select wrapper code is failing. We probably could fix this by terminating the process like in the other error cases in the same method (grep for the "failed to write to the unnamed pipe" error to see the code).

******************** TEST 'lldb :: tools/lldb-mi/target/target-select-so-path.test' FAILED ********************
Script:
--
: 'RUN: at line 3';   /home/teemperor/.llvm/rel-build/./bin/clang -o /home/teemperor/.llvm/rel-build/tools/lldb/lit/tools/lldb-mi/target/Output/target-select-so-path.test.tmp /home/teemperor/.llvm/llvm/tools/lldb/lit/tools/lldb-mi/target/inputs/main.c -g
: 'RUN: at line 4';   python /home/teemperor/.llvm/llvm/tools/lldb/lit/tools/lldb-mi/target/inputs/target-select-so-path.py "/home/teemperor/.llvm/rel-build/bin/lldb-server gdbserver" "/home/teemperor/.llvm/rel-build/bin/lldb-mi --synchronous /home/teemperor/.llvm/rel-build/tools/lldb/lit/tools/lldb-mi/target/Output/target-select-so-path.test.tmp" /home/teemperor/.llvm/llvm/tools/lldb/lit/tools/lldb-mi/target/target-select-so-path.test
--
Exit Code: 143

Command Output (stderr):
--
+ : 'RUN: at line 3'
+ /home/teemperor/.llvm/rel-build/./bin/clang -o /home/teemperor/.llvm/rel-build/tools/lldb/lit/tools/lldb-mi/target/Output/target-select-so-path.test.tmp /home/teemperor/.llvm/llvm/tools/lldb/lit/tools/lldb-mi/target/inputs/main.c -g
+ : 'RUN: at line 4'
+ python /home/teemperor/.llvm/llvm/tools/lldb/lit/tools/lldb-mi/target/inputs/target-select-so-path.py '/home/teemperor/.llvm/rel-build/bin/lldb-server gdbserver' '/home/teemperor/.llvm/rel-build/bin/lldb-mi --synchronous /home/teemperor/.llvm/rel-build/tools/lldb/lit/tools/lldb-mi/target/Output/target-select-so-path.test.tmp' /home/teemperor/.llvm/llvm/tools/lldb/lit/tools/lldb-mi/target/target-select-so-path.test
failed to write to the unnamed pipe: timed out
/home/teemperor/.llvm/rel-build/tools/lldb/lit/tools/lldb-mi/target/Output/target-select-so-path.test.script: line 2:  7284 Terminated              python /home/teemperor/.llvm/llvm/tools/lldb/lit/tools/lldb-mi/target/inputs/target-select-so-path.py "/home/teemperor/.llvm/rel-build/bin/lldb-server gdbserver" "/home/teemperor/.llvm/rel-build/bin/lldb-mi --synchronous /home/teemperor/.llvm/rel-build/tools/lldb/lit/tools/lldb-mi/target/Output/target-select-so-path.test.tmp" /home/teemperor/.llvm/llvm/tools/lldb/lit/tools/lldb-mi/target/target-select-so-path.test

--

[apolyakov]

Do you mean ConnectToRemote method from lldb/tools/lldb-server/lldb-gdbserver.cpp?

[teemperor]

Yes, the writeSocketIdToPipe(unnamed_pipe_fd, socket_id); fails in this method/file.

[apolyakov]

AFAIR, adding an exit(...) to ConnectToRemote won't solve this problem. The test will still be failing on Arch.

[teemperor]

This comment was removed by teemperor.

[teemperor]

Posting my mail here for the record:

I was more hoping it would at least turn the deadlock into a fail,
this way I could at least run the test suit.

Anyway, the actual issue is related Python 3: Arch Linux (and probably
some other distributions that "already" upgraded to Python 3 as
default) will launch Python 3 when the test script calls python ....
And for some reason in Python 3.7, we will not inherit our FD from our
pipe to the subprocess, which causes this strange behavior when write
to the unrelated FD number in ConnectToRemote. When I explicitly call
Python 2.7 from the test, everything runs as expected.

The python docs don't see to mention this change (and it seems like a
bug to me), so I'm open for ideas how to fix this properly. In any
case this problem doesn't block this review.

[teemperor]

AFAIR, adding an exit(...) to ConnectToRemote won't solve this problem. The test will still be failing on Arch.

I was more hoping it would at least turn the deadlock into a fail,
this way I could at least run the test suit.

Anyway, the actual issue is related Python 3: Arch Linux (and probably
some other distributions that "already" upgraded to Python 3 as
default) will launch Python 3 when the test script calls python ....
And for some reason in Python 3.7, we will not inherit our FD from our
pipe to the subprocess, which causes this strange behavior when write
to the unrelated FD number in ConnectToRemote. When I explicitly call
Python 2.7 from the test, everything runs as expected.

The python docs don't see to mention this change (and it seems like a
bug to me), so I'm open for ideas how to fix this properly. In any
case this problem doesn't block this review.

(I hope this comment shows up in Phabricator, as it seems
reviews.llvm.org is currently having some internal errors).
Am So., 23. Sep. 2018 um 01:03 Uhr schrieb Alexander Polyakov via
Phabricator <[e-mail]>:

apolyakov added a comment.

AFAIR, adding an exit(...) to ConnectToRemote won't solve this problem. The test will still be failing on Arch.

https://reviews.llvm.org/D52139

[apolyakov]

If so, we can try to run the script with python2.x. @teemperor can you try to modify target-select-so-path.test this way:
change # RUN: python %p/inputs/target-select-so-path.py "%debugserver" "%lldbmi %t" %s to
# RUN: python2 %p/inputs/target-select-so-path.py "%debugserver" "%lldbmi %t" %s

If this works for you on Arch, I'll update the revision.

[teemperor]

Yeah, explicitly typing python2 is what I did to fix it. Not sure if that breaks other OSs though (e.g. if a system has no python2 binary, but only python).

Also, that fix should be its own revision. It's not connected to the idea behind this revision from what I can see.

[apolyakov]

Reduced timer from 120 to 30 seconds.

[tatyana-krasnukha]

A lot of tests are failing with Python 3, at least on CentOS. So, I agree the problem doesn't block this review.

[labath]

In D52139#1243170, @teemperor wrote:

Anyway, the actual issue is related Python 3: Arch Linux (and probably
some other distributions that "already" upgraded to Python 3 as
default) will launch Python 3 when the test script calls python ....
And for some reason in Python 3.7, we will not inherit our FD from our
pipe to the subprocess, which causes this strange behavior when write
to the unrelated FD number in ConnectToRemote. When I explicitly call
Python 2.7 from the test, everything runs as expected.

The python docs don't see to mention this change (and it seems like a
bug to me), so I'm open for ideas how to fix this properly. In any
case this problem doesn't block this review.

It's more like python 2 had a bug where it always inherited the file descriptor, and then python3 fixed that and introduced a special popen argument to control the inheriting behavior.

Back when we were designing this test, I demonstrated the necessary incantations for this to work on both python2 and 3 https://reviews.llvm.org/D49739?id=157310#inline-438290. It seems that did not make it into the final version..

[apolyakov]

Thanks Pavel, I fixed it here https://reviews.llvm.org/D52498.

lldb/trunk/lit/tools/lldb-mi/target/inputs/target-select-so-path.py

 #!/usr/bin/env python2
 
 import os
 import sys
 import subprocess
+from threading import Timer
 
 
 hostname = 'localhost'
 
 (r, w) = os.pipe()
 args = sys.argv
 # Get debugserver, lldb-mi and FileCheck executables' paths with arguments.
 debugserver = ' '.join([args[1], '--pipe', str(w), hostname + ':0'])
 lldbmi = args[2]
 test_file = args[3]
 filecheck = 'FileCheck ' + test_file
 
 # Run debugserver, lldb-mi and FileCheck.
 debugserver_proc = subprocess.Popen(debugserver.split())
 lldbmi_proc = subprocess.Popen(lldbmi, stdin=subprocess.PIPE,
                                stdout=subprocess.PIPE, shell=True)
-filecheck_proc = subprocess.Popen(filecheck, stdin=lldbmi_proc.stdout,
+filecheck_proc = subprocess.Popen(filecheck, stdin=subprocess.PIPE,
                                   shell=True)
 
+timeout_sec = 30
+timer = Timer(timeout_sec, lldbmi_proc.kill)
+try:
+    timer.start()
+
     # Get a tcp port chosen by debugserver.
     # The number quite big to get lldb-server's output and to not hang.
     bytes_to_read = 10
     port_bytes = os.read(r, bytes_to_read)
     port = str(port_bytes.decode('utf-8').strip('\x00'))
 
     with open(test_file, 'r') as f:
         # Replace '$PORT' with a free port number and pass
         # test's content to lldb-mi.
         lldbmi_proc.stdin.write(f.read().replace('$PORT', port))
-    lldbmi_proc.wait()
-    filecheck_proc.wait()
+        out, err = lldbmi_proc.communicate()
+        filecheck_proc.stdin.write(out)
+finally:
+    timer.cancel()
 
 debugserver_proc.kill()
 exit(filecheck_proc.returncode)