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

Drop global DocList, attach the parsed result to Mailbox #1803

Merged
merged 1 commit into from Aug 19, 2019

Conversation

shadofren
Copy link

Drop the global DocList cache and attach DocList to Mailbox

  • What does this PR do?
    Before this change, help.c contains a global DocList containing parsed Email. As help documents are parsed only once. Caching is not necessary. Thus removing all caching logic and attach the DocList to Mailbox's mdata.

  • Screenshots (if relevant)

  • Does this PR meet the acceptance criteria? (This is just a reminder for you,
    this section can be removed if you fulfill it.)

  • What are the relevant issue numbers?

@shadofren
Copy link
Author

shadofren commented Aug 12, 2019

Hi,
I'm still having a hard time attaching doclist_free into mailbox's free_mdata.

Line 716: // m->free_mdata = help_doclist_free;

Thread 1 "neomutt" received signal SIGSEGV, Segmentation fault.
0x000055555562746d in mutt_addrlist_clear (al=0x1400000014) at address/address.c:1353
1353      if (!al)
#0  0x000055555562746d in mutt_addrlist_clear (al=0x1400000014) at address/address.c:1353
#1  0x000055555561e50e in mutt_env_free (p=p@entry=0x55555578d828) at email/envelope.c:67
#2  0x000055555561f632 in email_free (ptr=ptr@entry=0x7fffffffcd00) at email/email.c:51
#3  0x0000555555606d43 in help_doc_free (item=<optimized out>) at help/help.c:118
#4  0x000055555560849f in vector_free (v=v@entry=0x7fffffffcd40, item_free=item_free@entry=0x555555606d20 <help_doc_free>) at help/vector.c:48
#5  0x0000555555606df4 in help_doclist_free (data=<optimized out>) at help/help.c:131
#6  0x0000555555617f81 in mailbox_free (ptr=0x5555557532f0) at core/mailbox.c:64
#7  0x0000555555617dee in account_mailbox_remove (a=a@entry=0x555555753240, m=m@entry=0x0) at core/account.c:98
#8  0x0000555555617e9f in account_free (ptr=ptr@entry=0x7fffffffcdf8) at core/account.c:123
#9  0x0000555555618440 in neomutt_account_remove (n=n@entry=0x5555556c1910, a=a@entry=0x0) at core/neomutt.c:119
#10 0x00005555556184c1 in neomutt_free (ptr=0x555555689800 <NeoMutt>) at core/neomutt.c:70
#11 0x000055555556c14b in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at main.c:1270

I'm not sure what happens here. Please help

@shadofren shadofren requested a review from flatcap August 12, 2019 17:43
@flatcap
Copy link
Member

flatcap commented Aug 14, 2019

I think I see the problem.
The DocList and the Mailbox are sharing one array and they're both trying to free it.
Let me see if I can find the best way to sort this out.

Copy link
Member

@flatcap flatcap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've rebased, fixed the crashes and all the memory leaks.
Only some of the problems were yours, others you inherited (I've doc'd them all).

The main problem was the ownership of the array of Emails.
Now we clearly hand over control to the Mailbox.

help/help.c Show resolved Hide resolved
help/help.c Show resolved Hide resolved
help/help.c Show resolved Hide resolved
help/help.c Show resolved Hide resolved
help/help.c Show resolved Hide resolved
help/scan.c Show resolved Hide resolved
help/scan.c Show resolved Hide resolved
help/vector.c Show resolved Hide resolved
help/vector.c Show resolved Hide resolved
mx.c Show resolved Hide resolved
Drop the global DocList cache and attach DocList to Mailbox
Remove unused functions help_doclist_free() and help_doc_free().
@flatcap flatcap merged commit bdd179f into neomutt:devel/help Aug 19, 2019
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.

None yet

2 participants