Re: [PATCH] ruby: make sure the database is closed

diff --git a/e5/3a04533945abdee4b5fb0d65c2859d59f80070 b/e5/3a04533945abdee4b5fb0d65c2859d59f80070
new file mode 100644 (file)
index 0000000..5403843
--- /dev/null
+++ b/e5/3a04533945abdee4b5fb0d65c2859d59f80070
@@ -0,0 +1,176 @@
+Return-Path: <polatel@gmail.com>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+       by olra.theworths.org (Postfix) with ESMTP id 0179C431FAF\r
+       for <notmuch@notmuchmail.org>; Mon, 23 Apr 2012 16:46:50 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.699\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.699 tagged_above=-999 required=5\r
+       tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, FREEMAIL_FROM=0.001,\r
+       RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+       by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+       with ESMTP id 9Kxx090A319m for <notmuch@notmuchmail.org>;\r
+       Mon, 23 Apr 2012 16:46:49 -0700 (PDT)\r
+Received: from mail-bk0-f53.google.com (mail-bk0-f53.google.com\r
+       [209.85.214.53]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
+       (No client certificate requested)\r
+       by olra.theworths.org (Postfix) with ESMTPS id 7AC0C431FAE\r
+       for <notmuch@notmuchmail.org>; Mon, 23 Apr 2012 16:46:49 -0700 (PDT)\r
+Received: by bkcjm2 with SMTP id jm2so180939bkc.26\r
+       for <notmuch@notmuchmail.org>; Mon, 23 Apr 2012 16:46:47 -0700 (PDT)\r
+DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113;\r
+       h=mime-version:sender:in-reply-to:references:from:date\r
+       :x-google-sender-auth:message-id:subject:to:cc:content-type\r
+       :content-transfer-encoding;\r
+       bh=7xYckBiySqEncjt2n3rvMxKil5xmjoocg38cZLfrowg=;\r
+       b=HoRtfkTMJMCwV5OpbQjGg0DhKjihWus3xelGiG4z06RgXC4uCsGHtevqFaShD+hPf7\r
+       U3KR5iQ7MAgsmrpuIH3SZveXb7F8v3MBbU95EzmGgZei3+AHkU5BgUi1bBZXyuu+yoGe\r
+       PPasLtH1KNUJi+e6/Nz19EryPFml3hkMhxXMGSNutdUL0qLw/olG6EKnJFVqL7xpOyOX\r
+       u66gi1YJVjX8V2R05/7hayxbCOQAKwqjc707bJmOGfL81cm9KLDsbj4spoz9hOip108m\r
+       Y8IS2zROfkXNdONf7qnI6Q7c0y1LMK8khP45lPmU/YYw2dIHfyz8jf2PN7gLzM4kDsZe\r
+       jHfA==\r
+Received: by 10.204.152.14 with SMTP id e14mr5601257bkw.29.1335224807769; Mon,\r
+       23 Apr 2012 16:46:47 -0700 (PDT)\r
+MIME-Version: 1.0\r
+Sender: polatel@gmail.com\r
+Received: by 10.204.123.73 with HTTP; Mon, 23 Apr 2012 16:46:27 -0700 (PDT)\r
+In-Reply-To:\r
+ <CAMP44s0=m+PmVBdVytHaYujpaZu=2WH+1F_VoMzpfXH+SS_ZmQ@mail.gmail.com>\r
+References: <1335185032-13075-1-git-send-email-felipe.contreras@gmail.com>\r
+       <CADv3eywAvyMuh3vWLwyuf0Ui_kskwp9875pGxCR1GTm7deN9Pg@mail.gmail.com>\r
+       <CAMP44s3SyU4WVV0_McHWseNL=jmMnAXO2EdZK4Xk-wrCHPVD8A@mail.gmail.com>\r
+       <CADv3eywb0tguYowTAK5Ag9YZ48zFZA0QJVNEj_cZcCpr-76Bbg@mail.gmail.com>\r
+       <CAMP44s0=m+PmVBdVytHaYujpaZu=2WH+1F_VoMzpfXH+SS_ZmQ@mail.gmail.com>\r
+From: Ali Polatel <alip@exherbo.org>\r
+Date: Tue, 24 Apr 2012 02:46:27 +0300\r
+X-Google-Sender-Auth: Zf6lpMdc_sHRazsUQlgEXB3TYas\r
+Message-ID:\r
+ <CADv3eyxcu=2ZJ7GH3WULKMqe6FdmzhPtU6K9MLcCQ4m0cWmM7A@mail.gmail.com>\r
+Subject: Re: [PATCH] ruby: make sure the database is closed\r
+To: Felipe Contreras <felipe.contreras@gmail.com>\r
+Content-Type: text/plain; charset=ISO-8859-1\r
+Content-Transfer-Encoding: quoted-printable\r
+Cc: notmuch@notmuchmail.org\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+       <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Mon, 23 Apr 2012 23:46:51 -0000\r
+\r
+2012/4/24 Felipe Contreras <felipe.contreras@gmail.com>:\r
+> On Mon, Apr 23, 2012 at 11:43 PM, Ali Polatel <alip@exherbo.org> wrote:\r
+>> 2012/4/23 Felipe Contreras <felipe.contreras@gmail.com>:\r
+>>> On Mon, Apr 23, 2012 at 5:04 PM, Ali Polatel <alip@exherbo.org> wrote:\r
+>>>\r
+>>>> I'd rather not do this.\r
+>>>> Please read: http://comments.gmane.org/gmane.comp.lang.ruby.general/32=\r
+0324\r
+>>>\r
+>>> OK, I've read this.. So?\r
+>>\r
+>> You are one step close to what I thought you had thought.\r
+>\r
+> I don't parse that. I meant that _now_ I've read it.\r
+\r
+Sorry, my subconscious likes to barf on emails every now and then.\r
+\r
+>>> The order in which Ruby's garbage-collector frees the database and\r
+>>> other objects is irrelevant, because with this patch we are not\r
+>>> manually freeing other objects, only the database.\r
+>>\r
+>> What I wanted was to make all objects "depending" on the database to be =\r
+unusable\r
+>> once the database object is freed. Seeing that's not practically easy\r
+>> I decided to leave\r
+>> it to the user.\r
+>\r
+> Well, sure, but if the user has problems, the user can keep the\r
+> database object around.\r
+>\r
+> Personally I don't see why an object, like say a query would remain\r
+> working correctly after the database is gone, either by calling\r
+> .close() directly, or just loosing the pointer to the original object.\r
+> I don't think users would expect that, or, even if they somehow found\r
+> it useful, that most likely would be very seldom, and hardly worth\r
+> worrying about it.\r
+\r
+Working correctly is not expected but wouldn't it be more appropriate\r
+to throw an exception rather than dumping core or printing on standard erro=\r
+r?\r
+\r
+>>> Sure, it's _better_ if the user calls close(), even better if it's\r
+>>> inside an 'ensure', and even better if blocks are used (which I am\r
+>>> using in most cases), but that's not *required*.\r
+>>\r
+>> If you have such a use case, I'm fine with that patch.\r
+>> I might push it in the next few days or get someone else to push it.\r
+>\r
+> I did at some point, not sure if I do now. But that's beside the\r
+> point, the point is that it really does happen.\r
+>\r
+>>> The user might just do:\r
+>>>\r
+>>> def foo\r
+>>> =A0db =3D Notmuch::Database.new($db_name, :mode =3D> Notmuch::MODE_READ=\r
+_WRITE)\r
+>>> end\r
+>>>\r
+>>> That's perfectly fine in Ruby (although not ideal), since 'db' will\r
+>>> get garbage-collected. But nobody will be able to use the database\r
+>>> again until that process is killed.\r
+>>>\r
+>>> You think that's correct?\r
+>>\r
+>> Yes that is correct. I have not thought about this.\r
+>> I'd say it's a partial misunderstanding on my part due to lack of\r
+>> (and/or too much) vodka.\r
+>\r
+> Well, there's only two options.\r
+>\r
+> a) This works:\r
+>\r
+> =A0def foo\r
+> =A0 =A0db =3D Notmuch::Database.new($db_name, :mode =3D> Notmuch::MODE_RE=\r
+AD_WRITE)\r
+> =A0end\r
+>\r
+> (as in; the database gets closed eventually)\r
+>\r
+> b) This works:\r
+>\r
+> =A0def start_query(search)\r
+> =A0 =A0db =3D Notmuch::Database.new($db_name)\r
+> =A0 =A0return db.query(search)\r
+> =A0end\r
+>\r
+> (as in; the query returned would keep working properly)\r
+>\r
+> I personally don't see why anybody would want b), and if they have\r
+> problems with code like that, I think it's obvious to see why.\r
+>\r
+> I think we should go for a) and thus apply the patch.\r
+\r
+I wonder whether we can make both work somehow.\r
+Maybe by using talloc explicitly and keeping reference pointers?\r
+I don't know whether it's worth bothering.\r
+\r
+> Cheers.\r
+\r
+Cheers\r
+\r
+> --\r
+> Felipe Contreras\r
+\r
+        -alip\r