From patchwork Wed May 16 18:10:44 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 27303 Received: (qmail 48412 invoked by alias); 16 May 2018 18:10:48 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 48397 invoked by uid 89); 16 May 2018 18:10:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=hurt X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 16 May 2018 18:10:46 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 456D1406E897; Wed, 16 May 2018 18:10:45 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id C6EBB215ADD1; Wed, 16 May 2018 18:10:44 +0000 (UTC) Subject: Re: [PATCH 02/10] remote: Eliminate remote_hostio_close_cleanup To: Tom Tromey References: <20180516141830.16859-1-palves@redhat.com> <20180516141830.16859-3-palves@redhat.com> <87wow3h57t.fsf@tromey.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <61a8b543-5e01-bb52-09c3-298363c0bef8@redhat.com> Date: Wed, 16 May 2018 19:10:44 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <87wow3h57t.fsf@tromey.com> On 05/16/2018 06:37 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves writes: > > Pedro> + ~scoped_remote_fd () > Pedro> + { > Pedro> + if (m_fd != -1) > Pedro> + { > Pedro> + int remote_errno; > Pedro> + remote_hostio_close (find_target_at (process_stratum), > Pedro> + m_fd, &remote_errno); > > The only danger here is if remote_hostio_close can throw. However, this > was already a danger (in theory) before the patch -- there is a rule > (perhaps unwritten and/or unenforced) that one cannot throw during a > "do_cleanups". Yeah, a cleanup throwing leaves things in a bad state. I've been ignoring this whole "destructors shouldn't throw" thing in RAII-fying patches for that reason, but it wouldn't hurt to start handling it properly. I don't think there's anything that can be done in this case other than swallowing the exception. We already call remote_hostio_close explicitly in the normal paths, so the destructor trying to close the file is just for the already-unwinding-due-to-some-other-exception case. > I didn't look but some assurance that this can't happen would be good to > have. So it seems to me that the below would be the way to go. WDYT? diff --git a/gdb/remote.c b/gdb/remote.c index bc4815c67e..9761332ee4 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -12213,8 +12213,17 @@ public: if (m_fd != -1) { int remote_errno; - remote_hostio_close (find_target_at (process_stratum), - m_fd, &remote_errno); + try + { + remote_hostio_close (find_target_at (process_stratum), + m_fd, &remote_errno); + } + catch (...) + { + /* Swallow exception before it escapes the dtor. If + something goes wrong, likely the connection is gone, + and there's nothing else that can be done. */ + } } }