From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32478 invoked by alias); 10 Jan 2015 15:16:01 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 32466 invoked by uid 89); 10 Jan 2015 15:16:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 X-HELO: mail-pd0-f175.google.com Received: from mail-pd0-f175.google.com (HELO mail-pd0-f175.google.com) (209.85.192.175) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Sat, 10 Jan 2015 15:15:58 +0000 Received: by mail-pd0-f175.google.com with SMTP id g10so22767973pdj.6 for ; Sat, 10 Jan 2015 07:15:56 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; bh=0xG3Ln4DuE57dzrz79GgiFgDRTaU1kpkfqpsaa8tTgY=; b=An365DI3KGIEyJWhmT//XAdl1U580q8rXP+xssNlHIj+GDR3Gum4rkmVP5uiTDsMNv lktZgFT10+XEwC7aXGWLFl5GtCeUbVp3Vi6qzYl3jUTMxG00gsXTXCGWX+B9Qu7FaYq0 qNeBjvA8AggSteTaEqZgiMOvt4UPgesp3ictl7/9zPKHPifC9t/7dAKWe3TqQPqRuLUq WxoA0Cp86IPW20Lc4IIVNY5zz6n5/AaQqCryLnBGtEvoWq5UfvAnAj5r9JvxBR7qXTAV u28erV1nx7JTCs5SG2NLQpAw5+IICD8moZEzXRib3wRqIt1iH6gfFZ/j5H2oeSc1fG+q Kl8w== X-Gm-Message-State: ALoCoQl0qlmDqPTy9Xhh0v5efdHeOeZyQ1pRr6ov8+d+SaBsPA9K0BYPaOmi7sLt2cFJXAcSCwNp X-Received: by 10.70.133.35 with SMTP id oz3mr31995422pdb.69.1420902956649; Sat, 10 Jan 2015 07:15:56 -0800 (PST) MIME-Version: 1.0 Received: by 10.70.22.145 with HTTP; Sat, 10 Jan 2015 07:15:36 -0800 (PST) In-Reply-To: References: <1417226462-11254-1-git-send-email-patrick@parcs.ath.cx> <54808956.9070507@redhat.com> <54818CCE.5010701@redhat.com> <54887AB5.3000101@redhat.com> From: Patrick Palka Date: Sat, 10 Jan 2015 15:16:00 -0000 Message-ID: Subject: Re: [PATCH] Append to input history file instead of overwriting it To: Pedro Alves Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=UTF-8 X-SW-Source: 2015-01/txt/msg00257.txt.bz2 On Sat, Jan 10, 2015 at 9:09 AM, Patrick Palka wrote: > On Wed, Dec 10, 2014 at 11:54 AM, Pedro Alves wrote: >> On 12/05/2014 02:11 PM, Patrick Palka wrote: >>> On Fri, Dec 5, 2014 at 5:45 AM, Pedro Alves wrote: >>>> On 12/05/2014 12:19 AM, Patrick Palka wrote: >>>>> On Thu, 4 Dec 2014, Pedro Alves wrote: >>>>> >>>>>> On 11/29/2014 02:01 AM, Patrick Palka wrote: >>>>>>> This patch makes readline append new history lines to the GDB history >>>>>>> file on exit instead of overwriting the entire history file on exit. >>>>>>> This change allows us to run multiple simultaneous GDB sessions without >>>>>>> having each session overwrite the added history of each other session on >>>>>>> exit. It is particularly helpful when debugging GDB with GDB. >>>>>>> >>>>>>> Does this look OK to commit? Tested on x86_64-unknown-linux-gnu. >>>>>> >>>>>> Does this mean the history file will keep growing forever, rather than >>>>>> be trimmed by the history size? >>>>> >>>>> That it does... my .gdb_history is up to 2200 lines already! >>>>> >>>>> Looks like we have to explicitly truncate the history file on exit after >>>>> appending to it. Here's v2 of the patch that initializes the static >>>>> variable command_count, and calls history_truncate_file() appropriately. >>>>> Does it look OK? >>>> >>>> I'd like to hear how does bash (the canonical readline/history user) >>>> handle this scenario, if at all. >>> >>> It looks like bash truncates the history file size whenever the >>> $HISTFILESIZE variable is changed (which is usually at startup when it >>> reads ~/.bashrc), not on exit like this patch does. It doesn't do any >>> kind of file-level locking for the truncation operation or for >>> write_history() or append_history(). It writes directly to $HISTFILE. >> >> Bah. >> >> Is it that hard to do though? How about temporarily renaming the >> history file to something that includes gdb's PID (and would not a >> file name a user would use in practice) while we append >> to it, and then (atomically) rename it back? Something like: >> >> #1 - move $HISTFILE -> $HISTFILE-gdb$PID~ >> #2 - write/append history to $HISTFILE-gdb$PID~ >> #3 - move $HISTFILE-gdb$PID~ -> $HISTFILE >> >> We can then use non-atomic file access at will in #2, as no >> other process will be writing to that file (unless the user >> does evil thinks with "set history filename2, but then he deserves >> what he gets). >> >> This way, if two GDB's race writing to the file, then we'll lose the >> history of one of them, but that's better than ending up with a >> corrupted file, IMO. > > With your renaming method, what to do if the user has no .gdb_history > file to begin with? How would you tell the difference between the > case of having no .gdb_history and the case of another process in the > middle of writing to the history file (and thus having temporarily > renamed it)? In either case it looks like the .gdb_history file > doesn't exist. But in the first case we want to write a history file > anyway, and in the second case we want to give up on writing to the > history file. But it doesn't seem possible to tell the difference > between these two cases with your proposed method. .... therefore we must conservatively assume case #1, that the history file does not exist, and to write (not append) the process's known history to the local history file and try to rename it back anyway. > >> >>> >>>> It seems like we're opening ourselves >>>> up for more chances of history file corruption, considering that e.g., >>>> you may be quitting several gdb's simultaneously (e.g., when SIGTERM >>>> is sent to all processes). A single write call with O_APPEND should >>>> be atomic, but history_do_write uses mmap if available. And then >>>> seems like the truncation operation could end up with a broken hist >>>> file as well. >>>> ISTM that if we go this path, we should be doing an atomic update: >>>> create a new file under a parallel name, and then move to final destination. >>> >>> history_truncate_file() is definitely not atomic and does not look >>> obviously thread-safe, but if bash gets away with not doing any kind >>> of file-level locking, then can't we? :) >>> >>>> >>>>> This patch makes readline append new history lines to the GDB history >>>>> file on exit instead of overwriting the entire history file on exit. >>>> >>>>> This change allows us to run multiple simultaneous GDB sessions without >>>>> having each session overwrite the added history of each other session on >>>>> exit. >>>> >>>>> It is particularly helpful when debugging GDB with GDB. >>>> >>>> I'd like to have the description of how this helps that use case >>>> expanded. I mean, why would you want the history of the top >>>> level gdb mixed with the inferior gdb's history? Like, in: >>> >>> I don't necessarily want to, but I think such behavior is more useful >>> than not retaining the inferior gdb's history at all. Besides I don't >>> debug GDB that way. >>> >>> In one shell I open "gdb", in another I open "gdb -p $(pidof gdb)" and >>> I execute commands in both processes from time to time. In such a >>> scenario, the contents of the history file depends on which gdb >>> process I close first. I would rather like to have the history file >>> retain the commands of both processes. >> >> That still sounds odd to me -- the commands issued in the inferior gdb >> should be of no use to the other gdb, IMO. >> >>> This problem is not peculiar >>> to debugging GDB with GDB, rather it's encountered whenever there are >>> multiple GDB processes running simultaneously. >> >> Yes, that makes more sense. >> >>> So I would rather >>> remove that remark from the commit message ("It is particularly >>> helpful when debugging GDB with GDB.") because it's not really true. >> >> Alright. >> >> Thanks, >> Pedro Alves