f6/6479963ddb41c5fa8d63274d4b5dc2158379fc

   1 Return-Path: <teythoon@jade-hamburg.de>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 0E35D431E64\r
   6         for <notmuch@notmuchmail.org>; Wed, 29 Feb 2012 22:59:39 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: 0\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]\r
  12         autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id IOJmPwFFVzIA for <notmuch@notmuchmail.org>;\r
  16         Wed, 29 Feb 2012 22:59:36 -0800 (PST)\r
  17 Received: from mail.cryptobitch.de (cryptobitch.de [88.198.7.68])\r
  18         (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits))\r
  19         (No client certificate requested)\r
  20         by olra.theworths.org (Postfix) with ESMTPS id 8AAD0431FAE\r
  21         for <notmuch@notmuchmail.org>; Wed, 29 Feb 2012 22:59:36 -0800 (PST)\r
  22 Received: from mail.jade-hamburg.de (unknown [85.183.11.228])\r
  23         (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits))\r
  24         (No client certificate requested)\r
  25         by mail.cryptobitch.de (Postfix) with ESMTPSA id 00F2D2CACE3\r
  26         for <notmuch@notmuchmail.org>; Thu,  1 Mar 2012 07:59:34 +0100 (CET)\r
  27 Received: by mail.jade-hamburg.de (Postfix, from userid 401)\r
  28         id 6BB89DF2A7; Thu,  1 Mar 2012 07:59:33 +0100 (CET)\r
  29 Received: from thinkbox.jade-hamburg.de (unknown [10.1.1.153])\r
  30         (using TLSv1 with cipher AES256-SHA (256/256 bits))\r
  31         (No client certificate requested) (Authenticated sender: teythoon)\r
  32         by mail.jade-hamburg.de (Postfix) with ESMTPSA id 61D48DF2A2;\r
  33         Thu,  1 Mar 2012 07:59:31 +0100 (CET)\r
  34 Received: from teythoon by thinkbox.jade-hamburg.de with local (Exim 4.77)\r
  35         (envelope-from <teythoon@thinkbox.jade-hamburg.de>)\r
  36         id 1S2zz8-0000Su-Ht; Thu, 01 Mar 2012 07:59:30 +0100\r
  37 Content-Type: text/plain; charset="utf-8"\r
  38 MIME-Version: 1.0\r
  39 Content-Transfer-Encoding: quoted-printable\r
  40 To: Austin Clements <amdragon@MIT.EDU>, \r
  41 From: Justus Winter <4winter@informatik.uni-hamburg.de>\r
  42 In-Reply-To: <20120229221754.GD772@mit.edu>\r
  43 References:\r
  44  <1330507157-22859-1-git-send-email-4winter@informatik.uni-hamburg.de>\r
  45         <1330507157-22859-2-git-send-email-4winter@informatik.uni-hamburg.de>\r
  46         <20120229154833.GB772@mit.edu> <20120229211922.GQ24964@survex.com>\r
  47         <20120229221754.GD772@mit.edu>\r
  48 Message-ID: <20120301065930.5076.72748@thinkbox.jade-hamburg.de>\r
  49 User-Agent: alot/0.21+\r
  50 Subject: Re: [PATCH] Actually close the xapian database in\r
  51         notmuch_database_close\r
  52 Date: Thu, 01 Mar 2012 07:59:30 +0100\r
  53 Cc: Olly Betts <olly@survex.com>, notmuch@notmuchmail.org\r
  54 X-BeenThere: notmuch@notmuchmail.org\r
  55 X-Mailman-Version: 2.1.13\r
  56 Precedence: list\r
  57 List-Id: "Use and development of the notmuch mail system."\r
  58         <notmuch.notmuchmail.org>\r
  59 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  60         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  61 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  62 List-Post: <mailto:notmuch@notmuchmail.org>\r
  63 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  64 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  65         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  66 X-List-Received-Date: Thu, 01 Mar 2012 06:59:39 -0000\r
  67 \r
  68 Hi :)\r
  69 \r
  70 Olly wrote:\r
  71 >It is hard to say if calling close() is actually useful here from just\r
  72 >seeing the patch.\r
  73 \r
  74 Huh? I provided a test case...\r
  75 \r
  76 Quoting Austin Clements (2012-02-29 23:17:54)\r
  77 >Quoth Olly Betts on Feb 29 at  9:19 pm:\r
  78 >> On Wed, Feb 29, 2012 at 10:48:33AM -0500, Austin Clements wrote:\r
  79 >> > Quoth Justus Winter on Feb 29 at 10:19 am:\r
  80 >> > > Formerly the xapian database object was deleted and closed in its\r
  81 >> > > destructor once the object was garbage collected. Explicitly call\r
  82 >> > > close() so that the database and the associated lock is released\r
  83 >> > > immediately.\r
  84 >> > =\r
  85 \r
  86 >> > Interesting.  Is this a bug in Xapian?  According to the docs,\r
  87 >> > ~Database is supposed to close the database (if there are no other\r
  88 >> > copies, which there shouldn't be), so this should be redundant with\r
  89 >> > the delete notmuch->xapian_db a few lines down, but your experience\r
  90 >> > obviously suggests that it isn't and I can't find the code path in\r
  91 >> > Xapian that would close it in the destructor.\r
  92 >> =\r
  93 \r
  94 >> Most Xapian API classes (including Database and WritableDatabase) just\r
  95 >> hold a reference-counted pointer, and so it's the destructor of the\r
  96 >> reference-counted object which closes the database.  If "PIMPL" means\r
  97 >> anything to you, that's what we have here.\r
  98 >> =\r
  99 \r
 100 >> Some other API classes objects (such as PostingIterator) internally hold\r
 101 >> a reference to the database they are using, so calling close()\r
 102 >> explicitly is useful if you don't want to have to worry about such\r
 103 >> objects still existing and holding onto references which keep the\r
 104 >> database open.\r
 105 >\r
 106 >Makes sense.  Justus, could you add a comment to your patch explaining\r
 107 >that we explicitly close the database because there may be other\r
 108 >objects with references to it that would keep it open?\r
 109 \r
 110 I thought I did, I'm not a native speaker though, so if you want to\r
 111 reword my message be my guest ;)\r
 112 \r
 113 >Also, since close could throw an exception, it should get wrapped in a\r
 114 >try/catch like flush currently is.\r
 115 \r
 116 My interpretation of [0] was that Xapian::Database::close() does not\r
 117 throw any exceptions.\r
 118 \r
 119 Cheers,\r
 120 Justus\r
 121 \r
 122 0: http://xapian.org/docs/apidoc/html/classXapian_1_1Database.html#59f5f8b1=\r
 123 37723dcaaabdbdccbc0cf1eb\r