Prevent segmentation fault in notmuch_database_close

[Justus Winter]

I've seen some segmentation faults lately again and rechecked the
python bindings and while I actually found and fixed some error
handling issues I overlooked last time, I think I also found a bug in
notmuch_database_close.

The problem can easily be reproduced with the following program:

~~~ snip ~~~
import os
import time
import notmuch

db_path = os.path.expanduser('~/Maildir')

os.fork()
with notmuch.Database(db_path, mode=notmuch.Database.MODE.READ_WRITE) as db:
    time.sleep(5)
~~~ snap ~~~

The top of the resulting stack trace is

~~~ snip ~~~
#0  0x00007fa2b6ac8104 in Xapian::WritableDatabase::commit() ()
   from /usr/lib/libxapian.so.22
#1  0x00007fa2b7839de8 in Xapian::WritableDatabase::flush (this=0x0)
    at /usr/include/xapian/database.h:579
#2  0x00007fa2b78367aa in notmuch_database_close (notmuch=0x121ea70)
    at lib/database.cc:722
#3  0x00007fa2b7836749 in notmuch_database_open (path=
    0x7fa2b948ac94 "/home/teythoon/Maildir",
    mode=NOTMUCH_DATABASE_MODE_READ_WRITE) at lib/database.cc:705
~~~ snap ~~~

Cheers,
Justus

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Justus Winter]

Previously opening a notmuch database in read write mode that has been
locked resulted in the notmuch_database_open function executing
notmuch_database_close as a cleanup function. notmuch_database_close
failed to check whether the xapian database has in fact been created.

Add a check whether the xapian database object has actually been
created before trying to call its flush method.

Signed-off-by: Justus Winter <[hidden email]>
---
 lib/database.cc |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index c928d02..5efa85e 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -716,7 +716,8 @@ void
 notmuch_database_close (notmuch_database_t *notmuch)
 {
     try {
- if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE)
+ if (notmuch->xapian_db != NULL &&
+    notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE)
     (static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))->flush ();
     } catch (const Xapian::Error &error) {
  if (! notmuch->exception_reported) {
--
1.7.9

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Austin Clements]

Quoth Justus Winter on Feb 19 at 12:56 am:
> Previously opening a notmuch database in read write mode that has been
> locked resulted in the notmuch_database_open function executing
> notmuch_database_close as a cleanup function. notmuch_database_close
> failed to check whether the xapian database has in fact been created.
>
> Add a check whether the xapian database object has actually been
> created before trying to call its flush method.
>
> Signed-off-by: Justus Winter <[hidden email]>

LGTM.  Nice catch.

> ---
>  lib/database.cc |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index c928d02..5efa85e 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -716,7 +716,8 @@ void
>  notmuch_database_close (notmuch_database_t *notmuch)
>  {
>      try {
> - if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE)
> + if (notmuch->xapian_db != NULL &&
> +    notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE)
>      (static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))->flush ();
>      } catch (const Xapian::Error &error) {
>   if (! notmuch->exception_reported) {

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[David Bremner-2]

On Sun, 19 Feb 2012 00:56:57 +0100, Justus Winter <[hidden email]> wrote:
> Previously opening a notmuch database in read write mode that has been
> locked resulted in the notmuch_database_open function executing
> notmuch_database_close as a cleanup function. notmuch_database_close
> failed to check whether the xapian database has in fact been created.
>

Pushed.

d

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch