18/12405bbca1be073fc2767a8f83ef483ed0e4c1

   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 57402431FB6\r
   6         for <notmuch@notmuchmail.org>; Thu, 12 Apr 2012 10:20:49 -0700 (PDT)\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 DHZmjrBs27HI for <notmuch@notmuchmail.org>;\r
  16         Thu, 12 Apr 2012 10:20:48 -0700 (PDT)\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 66748431FAF\r
  21         for <notmuch@notmuchmail.org>; Thu, 12 Apr 2012 10:20:48 -0700 (PDT)\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 9DCF356A61B\r
  26         for <notmuch@notmuchmail.org>; Thu, 12 Apr 2012 19:20:46 +0200 (CEST)\r
  27 Received: by mail.jade-hamburg.de (Postfix, from userid 401)\r
  28         id 07F4FDF2A3; Thu, 12 Apr 2012 19:20:46 +0200 (CEST)\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 34EF2DF2A0;\r
  33         Thu, 12 Apr 2012 19:20:43 +0200 (CEST)\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 1SINgJ-0005ft-4i; Thu, 12 Apr 2012 19:19:39 +0200\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: <20120412165744.GF13549@mit.edu>\r
  43 References:\r
  44  <1332291311-28954-1-git-send-email-4winter@informatik.uni-hamburg.de>\r
  45         <20120401032323.GH5949@mit.edu>\r
  46         <20120412090533.2074.78211@thinkbox.jade-hamburg.de>\r
  47         <20120412165744.GF13549@mit.edu>\r
  48 Message-ID: <20120412171939.5852.22809@thinkbox.jade-hamburg.de>\r
  49 User-Agent: alot/0.21+\r
  50 Subject: Re: [RFC] Split notmuch_database_close into two functions\r
  51 Date: Thu, 12 Apr 2012 19:19:39 +0200\r
  52 Cc: notmuch@notmuchmail.org\r
  53 X-BeenThere: notmuch@notmuchmail.org\r
  54 X-Mailman-Version: 2.1.13\r
  55 Precedence: list\r
  56 List-Id: "Use and development of the notmuch mail system."\r
  57         <notmuch.notmuchmail.org>\r
  58 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  59         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  60 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  61 List-Post: <mailto:notmuch@notmuchmail.org>\r
  62 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  63 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  64         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  65 X-List-Received-Date: Thu, 12 Apr 2012 17:20:49 -0000\r
  66 \r
  67 Quoting Austin Clements (2012-04-12 18:57:44)\r
  68 >Quoth Justus Winter on Apr 12 at 11:05 am:\r
  69 >> Quoting Austin Clements (2012-04-01 05:23:23)\r
  70 >> >Quoth Justus Winter on Mar 21 at  1:55 am:\r
  71 >> >> I propose to split the function notmuch_database_close into\r
  72 >> >> notmuch_database_close and notmuch_database_destroy so that long\r
  73 >> >> running processes like alot can close the database while still using\r
  74 >> >> data obtained from queries to that database.\r
  75 >> >\r
  76 >> >Is this actually safe?  My understanding of Xapian::Database::close is\r
  77 >> >that, once you've closed the database, basically anything can throw a\r
  78 >> >Xapian exception.  A lot of data is retrieved lazily, both by notmuch\r
  79 >> >and by Xapian, so simply having, say, a notmuch_message_t object isn't\r
  80 >> >enough to guarantee that you'll be able to get data out of it after\r
  81 >> >closing the database.  Hence, I don't see how this interface could be\r
  82 >> >used correctly.\r
  83 >> =\r
  84 \r
  85 >> I do not know how, but both alot and afew (and occasionally the\r
  86 >> notmuch binary) are somehow safely using this interface on my box for\r
  87 >> the last three weeks.\r
  88 >\r
  89 >I see.  TL;DR: This isn't safe, but that's okay if we document it.\r
  90 >\r
  91 >The bug report [0] you pointed to was quite informative.  At its core,\r
  92 >this is really a memory management issue.  To sum up for the record\r
  93 >(and to check my own thinking): It sounds like alot is careful not to\r
  94 >use any notmuch objects after closing the database.  The problem is\r
  95 >that, currently, closing the database also talloc_free's it, which\r
  96 >recursively free's everything derived from it.  Python later GCs the\r
  97 >wrapper objects, which *also* try to free their underlying objects,\r
  98 >resulting in a double free.\r
  99 >\r
 100 >Before the change to expose notmuch_database_close, the Python\r
 101 >bindings would only talloc_free from destructors.  Furthermore, they\r
 102 >prevented the library from recursively freeing things at other times\r
 103 >by internally maintaining a reverse reference for every library talloc\r
 104 >reference (e.g., message is a sub-allocation of query, so the bindings\r
 105 >keep a reference from each message to its query to ensure the query\r
 106 >doesn't get freed).  The ability to explicitly talloc_free the\r
 107 >database subverts this mechanism.\r
 108 \r
 109 Exactly.\r
 110 \r
 111 >So, I've come around to thinking that splitting notmuch_database_close\r
 112 >and _destroy is okay.  It certainly parallels the rest of the API\r
 113 >better.  However, notmuch_database_close needs a big warning similar\r
 114 >to Xapian::Database::close's warning that retrieving information from\r
 115 >objects derived from this database may not work after calling close.\r
 116 \r
 117 Yes, but then again one should always expect function calls to fail\r
 118 and most APIs have mechanisms to communicate failures.\r
 119 \r
 120 OTOH this might be an indication that the notmuch API should be\r
 121 redesigned. Both alot and afew have their own wrappers around the\r
 122 notmuch API to work around some limitations (e.g. changes to messages\r
 123 are enqueued and executed at some point, with some kind of mechanism\r
 124 to cope with the notmuch database temporarily not being available,\r
 125 message objects have to be re-fetched if they got outdated (IIRC,\r
 126 whatever that means)).\r
 127 \r
 128 Justus\r