From patchwork Wed Jan 10 20:09:00 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 83784 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id ED6E43857C72 for ; Wed, 10 Jan 2024 20:09:36 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-il1-x133.google.com (mail-il1-x133.google.com [IPv6:2607:f8b0:4864:20::133]) by sourceware.org (Postfix) with ESMTPS id A31C53858407 for ; Wed, 10 Jan 2024 20:09:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A31C53858407 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A31C53858407 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::133 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704917351; cv=none; b=fisb2X0I93YoNCuyHwRhRdDrurTqCYbBZb5nHsX4nSZcHrNcT8zO4W6wB/pUJqRiS4XMaIco6d/sPaIhKqiAGzF1v9OFdZxqH9j38sgoLIJwrtZBI2uuzeZPrwFTtlfafdp0iX52k+ASmshKnRxc1aphOAjebW5a54WWFPv69v4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704917351; c=relaxed/simple; bh=X6Tco8jNJ5o1cLarcI9BYHIxOYs/DHC1R5Tb0qFLu+Y=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=l+JYcxhBc0HziTWQv3WOfiLA0M5Ndzm9DO1Je7Zi4zm8A4OjIOxYha3tvGQawudW8CJSTkEivZb5heaZurbeYVW8aAU93dUECkbN51oNVK5GWthlSdXoDzXLB/ehItSI1We3K16cxQneBMWNApxYz2csLBLLvpm1Ql6D7pcAxbQ= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-il1-x133.google.com with SMTP id e9e14a558f8ab-3606e69ec67so32102365ab.2 for ; Wed, 10 Jan 2024 12:09:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; t=1704917349; x=1705522149; darn=sourceware.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=RQQqH1lDQ6ITpyp4C4X5m33uhMGUCO7GPdbyGBtnkvw=; b=U4j+Uxmt/5LaHYpj4h6oadhYuDazFOr3IefF/dGOmOP38gfxDdMiem6nEDWQHTK5iY bRB9B1Fr0TzfBbcTK2ybJCMlcnpG63G/1M03eon/p42Cpap2Y8671Ww4AP71SE9CsxK3 3DjMvb9CBB4ShosKIzgUW2EPjsneewC9ZHJcg/+8QG6GLdCXeWmDDxrmaqf4df9vA0nQ 5FonkEnX5mTfnXqIJRGPxwl4GTT07Jh8dAT82edZ+arDbk+BKNbtNYTuPdyqi7Hn7rSB TQ96wdgy6HD3e2tv7uH4EQrMWTjICOk845XHlUdWfefM+tBrCxQUsvBbuUQLpB7dVyT1 YdwA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704917349; x=1705522149; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=RQQqH1lDQ6ITpyp4C4X5m33uhMGUCO7GPdbyGBtnkvw=; b=i2nxIckOjjTnF0F8G2XDqRbzpVSY17RKfIs7s75eu8x/lgUdZPJRFyLqj/Zx5poLFm DDa94qy300rAlUtaMUZGkamzDSOzi2OCnegK0MSAPtLffExIoUxUOUzrAgJ6rPOMTviF uXR4kW95EDDrVu64++80hHkXx6ORiwzaQeeCT2rBSlI/f3IM+Ku/yEf8vhmykZg6RR0K 6BMDgmWZxqRfS44oPG1s5J3p087lM4dVoH4cS+c2WJpm3px/MtOv/5P8HN0b/Kskuzxj YaD0Q89UiTcSG3yki5EMLFezv1z9oX3TA7+l8AR6MsagcRJarhC6ryVQciKeOARHKVl5 Gwkw== X-Gm-Message-State: AOJu0Yydoq13G0J11TI4/kREZsNlS3NbHP3JS3ErGvnWu0dmbiAYFIkL WlLvlFltZRmCqGnwrDVK2UJvzc2LEmJsY2IrWxKcJUBkMg== X-Google-Smtp-Source: AGHT+IEXBDB8aZVlIQDlFVC0VWfk27UD7yRC7sSfpz3J2Es9ILKdfKU24yLLeDWoZZzRS/uRWhO2qQ== X-Received: by 2002:a92:d1d0:0:b0:360:8937:53f9 with SMTP id u16-20020a92d1d0000000b00360893753f9mr63084ilg.88.1704917348770; Wed, 10 Jan 2024 12:09:08 -0800 (PST) Received: from localhost.localdomain (97-122-68-157.hlrn.qwest.net. [97.122.68.157]) by smtp.gmail.com with ESMTPSA id e12-20020a92690c000000b00360039199cesm1404064ilc.3.2024.01.10.12.09.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 10 Jan 2024 12:09:08 -0800 (PST) From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH] Handle EOF more gracefully in DAP Date: Wed, 10 Jan 2024 13:09:00 -0700 Message-ID: <20240110200900.791199-1-tromey@adacore.com> X-Mailer: git-send-email 2.43.0 MIME-Version: 1.0 X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org A user pointed out that gdb will print a Python exception when it gets an EOF in DAP mode. And, it turns out that an EOF like this also causes gdb not to exit. This is due to the refactoring that moved the JSON reader to its own thread -- previously this caused an exception to propagate and cause an exit, but now it just leaves the reader hung. This patch fixes these problems by arranging to handle EOF more gracefully. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31217 --- gdb/python/lib/gdb/dap/io.py | 54 +++++++++++++++++++------------ gdb/python/lib/gdb/dap/server.py | 7 ++++ gdb/testsuite/gdb.dap/eof.exp | 37 +++++++++++++++++++++ gdb/testsuite/lib/dap-support.exp | 4 +-- 4 files changed, 80 insertions(+), 22 deletions(-) create mode 100644 gdb/testsuite/gdb.dap/eof.exp diff --git a/gdb/python/lib/gdb/dap/io.py b/gdb/python/lib/gdb/dap/io.py index 2f3a2351925..367af481086 100644 --- a/gdb/python/lib/gdb/dap/io.py +++ b/gdb/python/lib/gdb/dap/io.py @@ -15,30 +15,44 @@ import json -from .startup import start_thread, send_gdb, log +from .startup import start_thread, send_gdb, log, log_stack, LogLevel def read_json(stream): """Read a JSON-RPC message from STREAM. - The decoded object is returned.""" - # First read and parse the header. - content_length = None - while True: - line = stream.readline() - line = line.strip() - if line == b"": - break - if line.startswith(b"Content-Length:"): - line = line[15:].strip() - content_length = int(line) - continue - log("IGNORED: <<<%s>>>" % line) - data = bytes() - while len(data) < content_length: - new_data = stream.read(content_length - len(data)) - data += new_data - result = json.loads(data) - return result + The decoded object is returned. + None is returned on EOF.""" + try: + # First read and parse the header. + content_length = None + while True: + line = stream.readline() + # If the line is empty, we hit EOF. + if len(line) == 0: + log("EOF") + return None + line = line.strip() + if line == b"": + break + if line.startswith(b"Content-Length:"): + line = line[15:].strip() + content_length = int(line) + continue + log("IGNORED: <<<%s>>>" % line) + data = bytes() + while len(data) < content_length: + new_data = stream.read(content_length - len(data)) + # Maybe we hit EOF. + if len(new_data) == 0: + log("EOF after reading the header") + return None + data += new_data + return json.loads(data) + except OSError: + # Reading can also possibly throw an exception. Treat this as + # EOF. + log_stack(LogLevel.FULL) + return None def start_json_writer(stream, queue): diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py index 59fad59d8e6..17a41564a92 100644 --- a/gdb/python/lib/gdb/dap/server.py +++ b/gdb/python/lib/gdb/dap/server.py @@ -187,6 +187,8 @@ class Server: def _reader_thread(self): while True: cmd = read_json(self.in_stream) + if cmd is None: + break log("READ: <<<" + json.dumps(cmd) + ">>>") # Be extra paranoid about the form here. If anything is # missing, it will be put in the queue and then an error @@ -201,6 +203,8 @@ class Server: ): self.canceller.cancel(cmd["arguments"]["requestId"]) self.read_queue.put(cmd) + # When we hit EOF, signal it with None. + self.read_queue.put(None) @in_dap_thread def main_loop(self): @@ -212,6 +216,9 @@ class Server: start_thread("JSON reader", self._reader_thread) while not self.done: cmd = self.read_queue.get() + # A None value here means the reader hit EOF. + if cmd is None: + break result = self._handle_command(cmd) self._send_json(result) events = self.delayed_events diff --git a/gdb/testsuite/gdb.dap/eof.exp b/gdb/testsuite/gdb.dap/eof.exp new file mode 100644 index 00000000000..139c17ad335 --- /dev/null +++ b/gdb/testsuite/gdb.dap/eof.exp @@ -0,0 +1,37 @@ +# Copyright 2024 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Test that EOF is handled gracefully. + +require allow_dap_tests + +load_lib dap-support.exp + +# The test doesn't matter much. +standard_testfile scopes.c + +if {[build_executable ${testfile}.exp $testfile $srcfile] == -1} { + return +} + +if {[dap_launch $testfile] == ""} { + return +} + +catch "close -i $gdb_spawn_id" +catch "wait -i $gdb_spawn_id" +unset gdb_spawn_id + +dap_check_log_file diff --git a/gdb/testsuite/lib/dap-support.exp b/gdb/testsuite/lib/dap-support.exp index bafcd9b6fc7..72b6ac537c3 100644 --- a/gdb/testsuite/lib/dap-support.exp +++ b/gdb/testsuite/lib/dap-support.exp @@ -339,7 +339,7 @@ proc dap_target_remote {target} { } # Read the most recent DAP log file and check it for exceptions. -proc _dap_check_log_file {} { +proc dap_check_log_file {} { global dap_gdb_instance set fd [open [standard_output_file "dap.log.$dap_gdb_instance"]] @@ -366,7 +366,7 @@ proc _dap_check_log_file {} { proc dap_shutdown {{terminate false}} { dap_check_request_and_response "shutdown" disconnect \ [format {o terminateDebuggee [l %s]} $terminate] - _dap_check_log_file + dap_check_log_file } # Search the event list EVENTS for an output event matching the regexp