This is an archive of the discontinued LLVM Phabricator instance.

Differential D65163

Fix occasional hangs of VSCode testcases
ClosedPublic

Authored by jankratochvil on Jul 23 2019, 12:05 PM.

Details

Reviewers
clayborg
labath
stella.stamenova
gkistanova
JDevlieghere
Commits
rG8693f8f9c182: [lldb] Fix occasional hangs of VSCode testcases
rL366850: [lldb] Fix occasional hangs of VSCode testcases
rLLDB366850: [lldb] Fix occasional hangs of VSCode testcases
Summary

On slower machines the vscode testcases were sometimes hanging:

1910 ?        Sl     0:00          |   \_ /usr/bin/python /home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/llvm/tools/lldb/test/dotest.py -q --arch=x86_64 -s /home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/lldb-test-traces --build-dir /home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/lldb-test-build.noindex -S nm -u CXXFLAGS -u CFLAGS --executable /home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/./bin/lldb --dsymutil /home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/./bin/dsymutil --filecheck /home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/./bin/FileCheck -C /home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/./bin/clang --env ARCHIVER=/usr/bin/ar --env OBJCOPY=/usr/bin/objcopy --env LLVM_LIBS_DIR=/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/./lib /home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/llvm/tools/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint -p TestVSCode_setBreakpoints.py
2649 ?        Sl     0:00          |       \_ /home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/bin/lldb-vscode
2690 ?        S      0:00          |           \_ /home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/bin/lldb-server gdbserver --fd=9 --native-regs --setsid
2708 ?        t      0:00          |               \_ /home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/lldb-test-build.noindex/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.test_functionality/a.out

Here is also a reproducer of the racy bug.
I guess request_continue was probably originally intended to be synchronous but then it isn't and this code has been leftover there.
I think it is an obvious fix but I have requested a review.

Diff Detail

Repository
rL LLVM

Event Timeline

jankratochvil created this revision.Jul 23 2019, 12:05 PM
Herald added a reviewer: JDevlieghere. · View Herald TranscriptJul 23 2019, 12:05 PM
Herald added a subscriber: lldb-commits. · View Herald Transcript
jankratochvil mentioned this in D65123: Restore tests for lldb-server and lldb-vscode removed at rL366590.Jul 23 2019, 12:07 PM
clayborg accepted this revision.Jul 23 2019, 12:46 PM

If all tests pass, then I am good with this.

This revision is now accepted and ready to land.Jul 23 2019, 12:46 PM
labath accepted this revision.Jul 23 2019, 12:47 PM

Thank you very much for tracking this down. Maybe wait a while to see if @clayborg has any thoughts on this, but otherwise, this looks good to me.

clayborg added a comment.Jul 23 2019, 1:00 PM
In D65163#1597902, @labath wrote:

Thank you very much for tracking this down. Maybe wait a while to see if @clayborg has any thoughts on this, but otherwise, this looks good to me.

haha, I already accepted. So good to go.

Closed by commit rL366850: [lldb] Fix occasional hangs of VSCode testcases (authored by jankratochvil). · Explain WhyJul 23 2019, 1:44 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 23 2019, 1:44 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
jankratochvil added a comment.Jul 23 2019, 1:47 PM

One heisenbug down!

Revision Contents

PathSize
lldb/
trunk/
packages/
Python/
lldbsuite/
test/
tools/
lldb-vscode/
8 lines

Diff 211353

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py

Show First 20 Lines • Show All 492 Lines • ▼ Show 20 Lines def request_continue(self, threadId=None):
threadId = self.get_thread_id() threadId = self.get_thread_id()
args_dict['threadId'] = threadId args_dict['threadId'] = threadId
command_dict = { command_dict = {
'command': 'continue', 'command': 'continue',
'type': 'request', 'type': 'request',
'arguments': args_dict 'arguments': args_dict
} }
response = self.send_recv(command_dict) response = self.send_recv(command_dict)
recv_packets = [] # Caller must still call wait_for_stopped.
self.recv_condition.acquire()
for event in self.recv_packets:
if event['event'] != 'stopped':
recv_packets.append(event)
self.recv_packets = recv_packets
self.recv_condition.release()
return response return response
def request_disconnect(self, terminateDebuggee=None): def request_disconnect(self, terminateDebuggee=None):
args_dict = {} args_dict = {}
if terminateDebuggee is not None: if terminateDebuggee is not None:
if terminateDebuggee: if terminateDebuggee:
args_dict['terminateDebuggee'] = True args_dict['terminateDebuggee'] = True
else: else:
▲ Show 20 Lines • Show All 587 Lines • Show Last 20 Lines