1 Return-Path: <amdragon@mit.edu>
\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 B170A431FAF
\r
6 for <notmuch@notmuchmail.org>; Thu, 12 Apr 2012 09:57:50 -0700 (PDT)
\r
7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org
\r
11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5
\r
12 tests=[RCVD_IN_DNSWL_LOW=-0.7] 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 Ks05ixbL1F5s for <notmuch@notmuchmail.org>;
\r
16 Thu, 12 Apr 2012 09:57:50 -0700 (PDT)
\r
17 Received: from dmz-mailsec-scanner-4.mit.edu (DMZ-MAILSEC-SCANNER-4.MIT.EDU
\r
19 by olra.theworths.org (Postfix) with ESMTP id E21AC431FAE
\r
20 for <notmuch@notmuchmail.org>; Thu, 12 Apr 2012 09:57:49 -0700 (PDT)
\r
21 X-AuditID: 1209190f-b7f8a6d000000914-bb-4f87098b16b6
\r
22 Received: from mailhub-auth-1.mit.edu ( [18.9.21.35])
\r
23 by dmz-mailsec-scanner-4.mit.edu (Symantec Messaging Gateway) with SMTP
\r
24 id 12.0B.02324.B89078F4; Thu, 12 Apr 2012 12:57:47 -0400 (EDT)
\r
25 Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])
\r
26 by mailhub-auth-1.mit.edu (8.13.8/8.9.2) with ESMTP id q3CGvkWV021095;
\r
27 Thu, 12 Apr 2012 12:57:47 -0400
\r
28 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])
\r
29 (authenticated bits=0)
\r
30 (User authenticated as amdragon@ATHENA.MIT.EDU)
\r
31 by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q3CGviOd024327
\r
32 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT);
\r
33 Thu, 12 Apr 2012 12:57:45 -0400 (EDT)
\r
34 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77)
\r
35 (envelope-from <amdragon@mit.edu>)
\r
36 id 1SINL6-0004IU-2r; Thu, 12 Apr 2012 12:57:44 -0400
\r
37 Date: Thu, 12 Apr 2012 12:57:44 -0400
\r
38 From: Austin Clements <amdragon@MIT.EDU>
\r
39 To: Justus Winter <4winter@informatik.uni-hamburg.de>
\r
40 Subject: Re: [RFC] Split notmuch_database_close into two functions
\r
41 Message-ID: <20120412165744.GF13549@mit.edu>
\r
43 <1332291311-28954-1-git-send-email-4winter@informatik.uni-hamburg.de>
\r
44 <20120401032323.GH5949@mit.edu>
\r
45 <20120412090533.2074.78211@thinkbox.jade-hamburg.de>
\r
47 Content-Type: text/plain; charset=us-ascii
\r
48 Content-Disposition: inline
\r
49 In-Reply-To: <20120412090533.2074.78211@thinkbox.jade-hamburg.de>
\r
50 User-Agent: Mutt/1.5.21 (2010-09-15)
\r
51 X-Brightmail-Tracker:
\r
52 H4sIAAAAAAAAA+NgFmphleLIzCtJLcpLzFFi42IR4hRV1u3mbPc3uDFZ32J26w8mi+s3ZzI7
\r
53 MHlMPH+azePZqlvMAUxRXDYpqTmZZalF+nYJXBl/Ln1jLNgpUfH2u1cDY4NIFyMnh4SAicTc
\r
54 tl5mCFtM4sK99WxdjFwcQgL7GCVu3XkI5WxglDhyfSY7hHOSSeLezqVMEM4SRonmnd3sIP0s
\r
55 AqoSl59tYQWx2QQ0JLbtX84IYosImEpsePAArIZZQFri2+9mJhBbWMBJYs2FSWBxXgEdiWln
\r
56 vkINXcUoMW3Tb6iEoMTJmU9YIJq1JG78ewlUxAE2aPk/DpAwp4CjxOOek2B7RQVUJKac3MY2
\r
57 gVFoFpLuWUi6ZyF0L2BkXsUom5JbpZubmJlTnJqsW5ycmJeXWqRropebWaKXmlK6iREc2JL8
\r
58 Oxi/HVQ6xCjAwajEw/viWZu/EGtiWXFl7iFGSQ4mJVHe1ezt/kJ8SfkplRmJxRnxRaU5qcWH
\r
59 GCU4mJVEeP88BirnTUmsrEotyodJSXOwKInzqmm98xMSSE8sSc1OTS1ILYLJynBwKEnwigMj
\r
60 WEiwKDU9tSItM6cEIc3EwQkynAdoOC9IDW9xQWJucWY6RP4Uo6KUOO9PDqCEAEgiozQPrheW
\r
61 eF4xigO9IswrBNLOA0xacN2vgAYzAQ3mUwC5urgkESEl1cBo8yDVwjU/fb6HVONxhSCHW58N
\r
62 P7uaFur1GKhI+3WGv9FRCl8WMX8D4w3eax1d2cuLtz2bphCQGNeY7nnHuDLD8ihT96+Ct73z
\r
63 Krg9b/1iOMVhmiK2eOeZRz8LVi399sPnzaNtTZbHLV9+YJgc/y0nI54nTlBp5ZcDbh3njRiv
\r
64 fI503pY8/6gSS3FGoqEWc1FxIgAexVgYFwMAAA==
\r
65 Cc: notmuch@notmuchmail.org
\r
66 X-BeenThere: notmuch@notmuchmail.org
\r
67 X-Mailman-Version: 2.1.13
\r
69 List-Id: "Use and development of the notmuch mail system."
\r
70 <notmuch.notmuchmail.org>
\r
71 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,
\r
72 <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>
\r
73 List-Archive: <http://notmuchmail.org/pipermail/notmuch>
\r
74 List-Post: <mailto:notmuch@notmuchmail.org>
\r
75 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>
\r
76 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,
\r
77 <mailto:notmuch-request@notmuchmail.org?subject=subscribe>
\r
78 X-List-Received-Date: Thu, 12 Apr 2012 16:57:50 -0000
\r
80 Quoth Justus Winter on Apr 12 at 11:05 am:
\r
81 > Quoting Austin Clements (2012-04-01 05:23:23)
\r
82 > >Quoth Justus Winter on Mar 21 at 1:55 am:
\r
83 > >> I propose to split the function notmuch_database_close into
\r
84 > >> notmuch_database_close and notmuch_database_destroy so that long
\r
85 > >> running processes like alot can close the database while still using
\r
86 > >> data obtained from queries to that database.
\r
88 > >Is this actually safe? My understanding of Xapian::Database::close is
\r
89 > >that, once you've closed the database, basically anything can throw a
\r
90 > >Xapian exception. A lot of data is retrieved lazily, both by notmuch
\r
91 > >and by Xapian, so simply having, say, a notmuch_message_t object isn't
\r
92 > >enough to guarantee that you'll be able to get data out of it after
\r
93 > >closing the database. Hence, I don't see how this interface could be
\r
96 > I do not know how, but both alot and afew (and occasionally the
\r
97 > notmuch binary) are somehow safely using this interface on my box for
\r
98 > the last three weeks.
\r
100 I see. TL;DR: This isn't safe, but that's okay if we document it.
\r
102 The bug report [0] you pointed to was quite informative. At its core,
\r
103 this is really a memory management issue. To sum up for the record
\r
104 (and to check my own thinking): It sounds like alot is careful not to
\r
105 use any notmuch objects after closing the database. The problem is
\r
106 that, currently, closing the database also talloc_free's it, which
\r
107 recursively free's everything derived from it. Python later GCs the
\r
108 wrapper objects, which *also* try to free their underlying objects,
\r
109 resulting in a double free.
\r
111 Before the change to expose notmuch_database_close, the Python
\r
112 bindings would only talloc_free from destructors. Furthermore, they
\r
113 prevented the library from recursively freeing things at other times
\r
114 by internally maintaining a reverse reference for every library talloc
\r
115 reference (e.g., message is a sub-allocation of query, so the bindings
\r
116 keep a reference from each message to its query to ensure the query
\r
117 doesn't get freed). The ability to explicitly talloc_free the
\r
118 database subverts this mechanism.
\r
121 So, I've come around to thinking that splitting notmuch_database_close
\r
122 and _destroy is okay. It certainly parallels the rest of the API
\r
123 better. However, notmuch_database_close needs a big warning similar
\r
124 to Xapian::Database::close's warning that retrieving information from
\r
125 objects derived from this database may not work after calling close.
\r
126 notmuch_database_close is really a specialty interface, and about the
\r
127 only thing you can guarantee after closing the database is that you
\r
128 can destroy other objects. This is also going to require a SONAME
\r
129 major version bump, as mentioned by others. Which, to be fair, would
\r
130 be a good opportunity to fix some other issues, too, like how
\r
131 notmuch_database_open can't return errors and how
\r
132 notmuch_database_get_directory is broken on read-only databases. The
\r
133 actual bump should be done at release time, but maybe we should drop a
\r
134 note somewhere (NEWS?) so we don't forget.
\r
136 [0] https://github.com/pazz/alot/issues/413
\r