Re: Usage after database close
David Bremner writes: >> But part of my question is, *should* this be improved? Am I >> interpreting notmuch's intended API correctly? > > Well, I agree you should get NOTMUCH_STATUS_XAPIAN_EXCEPTION back, or we > should change the docs to say "just don't do that". Arguments in favour of the latter: 1) several API calls don't return notmuch_status_t, so can't literally return NOTMUCH_STATUS_XAPIAN_EXCEPTION 2) notmuch_message_get_{message,thread}_id promise never to return NULL, has no way to report errors. I think it would probably make sense to say (if notmuch_database_reopen) existed, that if you call notmuch_database_close, don't call anything else except notmuch_database_reopen or notmuch_database_destroy. d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Usage after database close
Floris Bruynooghe writes: > > Ok, I forgot the "expected behaviour" part of the bug report ;) I think > that this doesn't work is fine and I'm not surprised by and your > description of fetching it first is very reasonable. However I was > expecting NOTMUCH_STATUS_XAPIAN_EXEPTION instead of bluntly getting > terminated. This is what the notmuch_database_close() docs say after > all. Sure, uncaught exceptions are never nice. > > I had a little look and this seems to be caused by the > message->doc.termlist_begin() call in > _notmuch_message_ensure_metadata(), I guess almost every Xapian API call will fail with the database closed. > I didn't have xapian debug symbols and am not familiar with xapian to > quickly have an idea of whether this case can be improved or not. > (-dbg debian packages for notmuch and xapian would be very handy ;)) > You need to add a seperate repo for the new style debug symbols in Debian: $ (git)-[master]-% apt policy libxapian30-dbgsym libxapian30-dbgsym: Installed: 1.4.15-1 Candidate: 1.4.15-1 Version table: *** 1.4.15-1 500 500 http://debug.mirrors.debian.org/debian-debug testing-debug/main amd64 Packages 500 http://debug.mirrors.debian.org/debian-debug unstable-debug/main amd64 Packages 100 /var/lib/dpkg/status > But part of my question is, *should* this be improved? Am I > interpreting notmuch's intended API correctly? Well, I agree you should get NOTMUCH_STATUS_XAPIAN_EXCEPTION back, or we should change the docs to say "just don't do that". d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Usage after database close
On Sun 28 Jun 2020 at 13:19 -0300, David Bremner wrote: > Floris Bruynooghe writes: > >> Hi, >> >> I started writing some test cases to define better what you can do with >> a closed database and make sure that the python bindings do not behave >> unexpectedly here too. >> >> One of the first things I tried ends up with xapian calling >> exit_group(2) directly, terminating the process. So I'm wondering if >> I'm approaching this entirely the wrong way. My understanding is that >> we should generally be allowed to use anything after the database has >> been closed, as long as nothing has been destroyed. >> >> Below is a minimal reproducible example of what I'm testing so far. I >> must admit I'm generally lazy here and usually just test with notmuch >> that is currently in Debian testing. > > Funny that you should mention lazy, that's basically what the problem is > here ;). notmuch_message_get_message_id is lazily trying to read the > information from the database. This is a bit surprising here because of > the query, but that's not really visible once the message object is > created. > > In principle it could be documented what parts of the API can trigger > access to the database, but I'm not sure about the benefit of the extra > complexity. It might be safer to assume that only access to already > fetched information is safe. In particular if you move > > messageid = notmuch_message_get_message_id(msg); > > before you close the database, then printing it out afterwards works. I > didn't run it valgrind to make sure, but afaik, that should be perfectly > legal. Ok, I forgot the "expected behaviour" part of the bug report ;) I think that this doesn't work is fine and I'm not surprised by and your description of fetching it first is very reasonable. However I was expecting NOTMUCH_STATUS_XAPIAN_EXEPTION instead of bluntly getting terminated. This is what the notmuch_database_close() docs say after all. I had a little look and this seems to be caused by the message->doc.termlist_begin() call in _notmuch_message_ensure_metadata(), I didn't have xapian debug symbols and am not familiar with xapian to quickly have an idea of whether this case can be improved or not. (-dbg debian packages for notmuch and xapian would be very handy ;)) But part of my question is, *should* this be improved? Am I interpreting notmuch's intended API correctly? > The original motivation (see 7864350c938944276c1a378539da7670c211b9b5) > to allow long running processes to release the lock on the > database. This is not a pattern we use in the CLI, so it's not as well > tested as it could be. In particular the work to export > notmuch_database_reopen (tests, documentation) has not happened yet. > > d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Usage after database close
Floris Bruynooghe writes: > Hi, > > I started writing some test cases to define better what you can do with > a closed database and make sure that the python bindings do not behave > unexpectedly here too. > > One of the first things I tried ends up with xapian calling > exit_group(2) directly, terminating the process. So I'm wondering if > I'm approaching this entirely the wrong way. My understanding is that > we should generally be allowed to use anything after the database has > been closed, as long as nothing has been destroyed. > > Below is a minimal reproducible example of what I'm testing so far. I > must admit I'm generally lazy here and usually just test with notmuch > that is currently in Debian testing. Funny that you should mention lazy, that's basically what the problem is here ;). notmuch_message_get_message_id is lazily trying to read the information from the database. This is a bit surprising here because of the query, but that's not really visible once the message object is created. In principle it could be documented what parts of the API can trigger access to the database, but I'm not sure about the benefit of the extra complexity. It might be safer to assume that only access to already fetched information is safe. In particular if you move messageid = notmuch_message_get_message_id(msg); before you close the database, then printing it out afterwards works. I didn't run it valgrind to make sure, but afaik, that should be perfectly legal. The original motivation (see 7864350c938944276c1a378539da7670c211b9b5) to allow long running processes to release the lock on the database. This is not a pattern we use in the CLI, so it's not as well tested as it could be. In particular the work to export notmuch_database_reopen (tests, documentation) has not happened yet. d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Usage after database close
Hi, I started writing some test cases to define better what you can do with a closed database and make sure that the python bindings do not behave unexpectedly here too. One of the first things I tried ends up with xapian calling exit_group(2) directly, terminating the process. So I'm wondering if I'm approaching this entirely the wrong way. My understanding is that we should generally be allowed to use anything after the database has been closed, as long as nothing has been destroyed. Below is a minimal reproducible example of what I'm testing so far. I must admit I'm generally lazy here and usually just test with notmuch that is currently in Debian testing. Cheers, Floris Here the script: #!/bin/sh MAILDIR=$(mktemp --directory) export MAILDIR echo $MAILDIR mkdir $MAILDIR/tmp mkdir $MAILDIR/new mkdir $MAILDIR/cur cat > $MAILDIR/notmuch-config < $MAILDIR/cur/1593342032.M818430P304029Q3.powell < Date: Sun, 28 Jun 2020 13:00:32 - From: s...@example.com To: d...@example.com Subject: Test mail Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 This is a test mail EOF notmuch new cat >$MAILDIR/test.c < #include int main(int argc, char** argv) { notmuch_status_t status; notmuch_database_t* db; notmuch_message_t* msg; const char* messageid; printf("Opening db\n"); status = notmuch_database_open(argv[1], NOTMUCH_DATABASE_MODE_READ_ONLY, &db); if (status != NOTMUCH_STATUS_SUCCESS) { return status; } printf("Finding msg\n"); status = notmuch_database_find_message(db, "0...@powell.devork.be", &msg); if (status != NOTMUCH_STATUS_SUCCESS) { return status; } printf("Closing db\n"); status = notmuch_database_close(db); if (status != NOTMUCH_STATUS_SUCCESS) { return status; } printf("Get messageid\n"); messageid = notmuch_message_get_message_id(msg); if (messageid == NULL) { return 1; } printf("Messageid: %s\n", messageid); printf("The end.\n"); } EOF gcc $MAILDIR/test.c -lnotmuch -o $MAILDIR/test $MAILDIR/test $MAILDIR ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: crypto test failures on Fedora and OpenSUSE
David Bremner writes: > I poked at this a bit more in gdb. I don't really know the code well, > but it seems like whatever is happening is happening inside gmime. On > the other hand both Fedora32 and my current Debian testing are running > libgmime 3.2.7 and gpgme 1.13 > I dug a bit further down, and this is what is returned from gpgme (line 345 in g_mime_gpgme_get_signatures) sig = {next = 0x0, summary = GPGME_SIGSUM_KEY_MISSING, fpr = 0x4ac480 "5AEAB11F5E33DCE875DDB75B6D92612D94E46381", status = 9, notations = 0x0, timestamp = 1559167762, exp_timestamp = 0, wrong_key_usage = 0, pka_trust = 0, chain_model = 0, is_de_vs = 0, _unused = 0, validity = GPGME_VALIDITY_UNKNOWN, validity_reason = 0, pubkey_algo = GPGME_PK_RSA, hash_algo = GPGME_MD_SHA256, pka_address = 0x0, key = 0x0} At this point I'm leaning towards declaring it a gpgme problem in fedora32, and suggesting that relevant distros mark the test broken. I am of course open to more informed opinions. d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch