-
-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
astroid crashes with nonsensical query #132
Comments
I cannot reproduce this, please post log of the output, and if you have the coredump post the stacktrace:
|
The log is exactly like when everything works fine. The backtrace is here:
I just installed astroid in Arch in VirtualBox with an empty notmuch database. I opened astroid, pressed F, pressed the up arrow key and obtained a Segmentation fault. Maybe there is some error with the code that handles the search history. |
Duplicate of #130. Something weird is going on here. |
Does it happen if you upgrade all libraries (-Syu) and re-compile astroid? |
Yes. It took several attempts, maybe 30. |
I've tried the new code, and was able to reproduce the crash with the following procedure:
|
apm256 writes on mai 13, 2016 10:10:
I finally found a way to reproduce this, I think it should be fixed now. |
Seems fixed, indeed. |
I am sorry to tell you that astroid still crashes. Here is a complete backtrace:
|
This is a different problem. I am closing this issue. If you identify what caused the new crash, please open a new issue. |
I obtained this crash doing precisely what I described in the first post of
this thread.
Looks like there is more than one thing that breaks with this query. I
would not close the issue until it is really solved.
|
What version of notmuch are you running? |
The first crash was not related to the query, but to how search-history was browsed. It crashed when you press up or down in the search bar. The second crash seems related to the specific query. |
I am using notmuch 0.22. I tried this query in notmuch and obtained the following:
This makes me believe that astroid is not handling the Xapian exception correctly. |
Marduk Bolaños writes on mai 18, 2016 21:04:
Please test newest git, I am now checking the status output. Astroid does not have access to the exceptions through the Notmuch C |
I tested the newest git. It took me 5 attempts to crash it. Hopefully something can be done to fix this. The exception is generated whenever you enter a query that notmuch does not understand. For example, when there is a typo in a prefix (date, from, etc.). |
Marduk Bolaños writes on mai 18, 2016 22:44:
I need to know exactly which queries crash. The from:XXXX.. crash is now |
I did some testing and arrived at the conclusion that every query that
includes an invalid range expression will crash astroid.
My suggestion would be to pre-process every query before sending it to
notmuch. If the string ".." is found, make sure that it belongs to a
valid date expression as specified in `man notmuch-search-terms`. Then
decide if the expression will be sent to notmuch or not.
|
Marduk Bolaños writes on mai 19, 2016 21:44:
It is not a good idea to start pre-processing the queries and fixing What exact query are you trying now? With the newest commit on the |
I totally agree that this has to be fixed in notmuch. However, I do not
expect that to happen anytime soon. The threading issue that I reported
here has been known to them since 2014 and it is still waiting for a
fix.
I updated to the latest commit and managed to crash astroid with the
query `dat:2015..`.
It is still not clear to me why does astroid crash. I have not looked
into the code, but my guess is that when the query is successful
libnotmuch returns some data type and when it is not, it returns another
data type. I would expect a crash if you feed the data returned by
libnotmuch directly to Gtk without verifying that the types match. But,
this is just speculation.
|
Marduk Bolaños writes on mai 20, 2016 0:37:
This is different. I have generally found notmuch to be fairly active.
Please update and try again, I added some more checks. If you have other
No, this is incorrect. If you have a look at the last commits and the |
IMHO notmuch should check that a query is valid before sending it to Xapian (and reject it when it is invalid), instead of waiting for things to blow up on the Xapian side.
I updated to the latest commit and crashed astroid with |
Marduk Bolaños writes on mai 20, 2016 20:11:
No - parsing and checking the query beforehand is difficult and
It is a bad idea to validate the query on the a) application side, or Please try again the latest commit, I forgot a check. So far it seems |
but we cannot implement it better or anticipate all the ingenious ways
a user may enter a faulty query.
I apologize for being so persistent with this, but it really interests
me and I want to learn. I appreciate your patience.
From my naive point of view, if you know exactly what is the structure
of a valid query then anything that does not comply with it is by
definition an invalid query. Therefore, you do not have to care about
all the possible ways to formulate an invalid query.
My naive implementation of a query parser would do this. Given a query,
search for "words" delimited by quotes (these are strings of words
separated by spaces), store them in a list and remove them from the
query. Given the new query split it at every occurrence of the space
character. This results in a list of words. Given a word search for a
colon character. If you find it, then split the word in a left and a
right part. In the left part you will only accept words that belong to
the list of prefixes. If the left part is date, then on the right part
you will only accept dates and ranges that conform to the formats
specified in the manual. If the left part is not date, then on the right
part you will not interpret .. as a range.
This very likely does not cover all cases, but so far is it reasonable?
Please try again the latest commit, I forgot a check.
I tried the latest commit and it crashed again with the query
`tag:flagged dat:2015..`.
|
Marduk Bolaños writes on mai 20, 2016 22:47:
But it is totally pointless when there already is a complete, tested, Parsers are hard to get right, it would require a lot of work to make it What do you think would be the difference in behaviour if we implemented it
What is the backtrace? Please send the log as well. |
You are right. Xapian is doing its job and it does it well.
I spent some hours researching how do the queries work all the way from
astroid through notmuch to xapian. It seems to me that the current
programming practices in C++ lead to very fragile and hard to understand
code. For example, a lot of code depends on implicit behavior rather
than explicitly stating what is to be done. It is difficult to
understand a piece of code unless you have in mind all the things that
are assumed.
I have read and written some Lisp and Racket and the code is much easier
to understand, potentially leading to less bugs. I think it is true that
learning some Lisp makes you a better programmer in any language.
What is the backtrace? Please send the log as well.
#0 0x00007ff9a7e64295 in raise () from /usr/lib/libc.so.6
#1 0x00007ff9a7e656da in abort () from /usr/lib/libc.so.6
#2 0x00007ff9a77188ac in ?? () from /usr/lib/libtalloc.so.2
#3 0x00007ff9a771846f in _talloc_free () from /usr/lib/libtalloc.so.2
#4 0x00000000004ad862 in ?? ()
#5 0x00007ff9a86c0aaf in std::execute_native_thread_routine (__p=0x7ff9aea41730) at /build/gcc/src/gcc/libstdc++-v3/src/c++11/thread.cc:83
#6 0x00007ff9a81d9474 in start_thread () from /usr/lib/libpthread.so.0
#7 0x00007ff9a7f18acd in clone () from /usr/lib/libc.so.6
Thank you very much for your time and efforts.
|
Marduk Bolaños writes on mai 21, 2016 14:52:
True, there is some weird things going on some times. But C++ interfaces
Please send the log as well. This is with the exact query: "tag:flagged dat:2015.." ? Does it crash This crash is in notmuch, but it might be fixed in notmuch-git. |
By the way: Make sure you have the latest git version of astroid |
Gaute Hope writes on mai 21, 2016 17:42:
By the way: https://github.com/gauteh/astroid/wiki/Style |
I tried with the latest commit and this query no longer crashes. Let's
hope that we can forget about this issue.
Thanks for your time and your support.
|
I just had the same problem as described in this thread with astroid 0.8. When I search for If you cannot reproduce it, and you need any additional information from me, please let me know. |
Matthias Kirschner writes on april 18, 2017 20:59:
I just had the same problem as described in this thread with astroid 0.8. When I search for
`dat:2015..` it crashed. When I restarted astroid `dat:2015..` said "no mails", when I did it a second tme with the same search it crashed again.
If you cannot reproduce it, and you need any additional information from me, please let me know.
It was fixed in notmuch, do you have an old notmuch around by any
chance?
|
My notmuch version is 0.24.1-2. So that should not be the problem, right? |
Matthias Kirschner writes on april 20, 2017 8:41:
My notmuch version is 0.24.1-2. So that should not be the problem, right?
No, should be fixed in that one. Not sure if this is correct behavior on
the notmuch side, but it should be fixed/avoided in the latest astroid-master
now.
|
By accident I discovered that performing the following query a few times (one after the other) crashes astroid:
tag:flagged from:2015..
I am using version v0.5.r164.ged2256a compiled from git in Arch.
The text was updated successfully, but these errors were encountered: