Skip to content
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

Fix segfault on opening multiple threads for one entry in thread list #696

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

matthiasbeyer
Copy link
Contributor

This is my take on #695

I'm not a C++ dev though (mainly I'm a Rust dev), thus I guess this is something one might not want to have here.

For my case, it fixes the crash when opening a thread where notmuch returns multiple threads for one thread id.
Please have a look.

This is necessary if we want to execute the closure for each found
thread.

Signed-off-by: Matthias Beyer <[email protected]>
@matthiasbeyer
Copy link
Contributor Author

Note that this does not open multiple new panes, one for each thread returned by notmuch. I would have expected that behaviour, but it does not do that. I don't know why.

@gauteh
Copy link
Member

gauteh commented Jul 22, 2020

Thanks, Only one additional pane is show

@gauteh
Copy link
Member

gauteh commented Jul 22, 2020

on_thread is used several places, not sure if it is safe to apply the function to all threads. E.g. a user-defined hook to delete a thread. Why does notmuch return several threads? Is it a notmuch bug? Maybe it is safer to work on the first thread if we cannot avoid the underlying issue?

@matthiasbeyer
Copy link
Contributor Author

TBH I don't know. But I've experienced this before in notmuch. As far as I can see, nothing (in the notmuch docs) says that for a thread:<thread id> query, notmuch does only return one thread.

So I guess it is actually expected to return multiple thread and the bug is on our side here when we expect only one thread to be returned.

@gauteh
Copy link
Member

gauteh commented Jul 22, 2020 via email

@matthiasbeyer
Copy link
Contributor Author

@gauteh
Copy link
Member

gauteh commented Jul 22, 2020 via email

@matthiasbeyer
Copy link
Contributor Author

Does the threads always contain the same messages?

Not sure what you mean by that.

From what I see in the codebase right now, Db::on_thread is not used too extensively. Maybe it would be worth a try to check all cases and rewrite it to not expect there be only one thread anymore?

@gauteh
Copy link
Member

gauteh commented Jul 22, 2020 via email

@mreppen
Copy link

mreppen commented Aug 5, 2020

I have solved this problem by running notmuch reindex on the first of such split threads. This corrects it to one thread. Is there a reason this could not be attempted by astroid as a convenience to the user? Or does reindexing potentially cause problems?

@matclab
Copy link

matclab commented Jan 7, 2021

I have solved this problem by running notmuch reindex on the first of such split threads. This corrects it to one thread. Is there a reason this could not be attempted by astroid as a convenience to the user? Or does reindexing potentially cause problems?

Thanks it has solved a similar problem here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants