unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] lib/message-file: close stream in destructor
@ 2019-05-10  0:23 David Bremner
  2019-05-10  1:05 ` Will Dietz
  2019-05-10 15:33 ` David Bremner
  0 siblings, 2 replies; 4+ messages in thread
From: David Bremner @ 2019-05-10  0:23 UTC (permalink / raw)
  To: notmuch

Without this,

$ make time-test OPTIONS=--small

leads to fatal errors from too many open files.

Thanks to st-gourichon-fid for bringing this problem to my attention in IRC.
---
 lib/message-file.c | 3 +++
 1 file changed, 3 insertions(+)

The regression test could be improved, but I wanted to get a fix out
and hopefully into master, as this is a problem that seems
particularly likely to hit new users during their initial notmuch-new.

diff --git a/lib/message-file.c b/lib/message-file.c
index 50855067..24c5fda4 100644
--- a/lib/message-file.c
+++ b/lib/message-file.c
@@ -46,6 +46,9 @@ _notmuch_message_file_destructor (notmuch_message_file_t *message)
     if (message->message)
 	g_object_unref (message->message);
 
+    if (message->stream)
+	g_object_unref (message->stream);
+
     return 0;
 }
 
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] lib/message-file: close stream in destructor
  2019-05-10  0:23 [PATCH] lib/message-file: close stream in destructor David Bremner
@ 2019-05-10  1:05 ` Will Dietz
  2019-05-10  1:42   ` David Bremner
  2019-05-10 15:33 ` David Bremner
  1 sibling, 1 reply; 4+ messages in thread
From: Will Dietz @ 2019-05-10  1:05 UTC (permalink / raw)
  To: notmuch

[-- Attachment #1: Type: text/plain, Size: 1480 bytes --]

Great, thank you!

Much appreciated, I actually was running into this yesterday.

( which for whatever reason led to posting this on the subject:
https://github.com/afewmail/afew/issues/193#issuecomment-490766850 )

Would the valgrind support in the test suite catch this sort of thing
(fd leak on relatively common path)?

Regardless, glad to see it fixed!

~Will

On Thu,  9 May 2019 21:23:24 -0300, David Bremner <david@tethera.net> wrote:
> Without this,
> 
> $ make time-test OPTIONS=--small
> 
> leads to fatal errors from too many open files.
> 
> Thanks to st-gourichon-fid for bringing this problem to my attention in IRC.
> ---
>  lib/message-file.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> The regression test could be improved, but I wanted to get a fix out
> and hopefully into master, as this is a problem that seems
> particularly likely to hit new users during their initial notmuch-new.
> 
> diff --git a/lib/message-file.c b/lib/message-file.c
> index 50855067..24c5fda4 100644
> --- a/lib/message-file.c
> +++ b/lib/message-file.c
> @@ -46,6 +46,9 @@ _notmuch_message_file_destructor (notmuch_message_file_t *message)
>      if (message->message)
>  	g_object_unref (message->message);
>  
> +    if (message->stream)
> +	g_object_unref (message->stream);
> +
>      return 0;
>  }
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

[-- Attachment #2: Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] lib/message-file: close stream in destructor
  2019-05-10  1:05 ` Will Dietz
@ 2019-05-10  1:42   ` David Bremner
  0 siblings, 0 replies; 4+ messages in thread
From: David Bremner @ 2019-05-10  1:42 UTC (permalink / raw)
  To: Will Dietz, notmuch

Will Dietz <w@wdtz.org> writes:

> Great, thank you!
>
> Much appreciated, I actually was running into this yesterday.
>
> ( which for whatever reason led to posting this on the subject:
> https://github.com/afewmail/afew/issues/193#issuecomment-490766850 )
>
> Would the valgrind support in the test suite catch this sort of thing
> (fd leak on relatively common path)?
>

Good question. It didn't in a naive test I tried (just running notmuch
new under valgrind --track-fds). This is probably has to do with glib
cleaning up the relevant objects during shutdown.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] lib/message-file: close stream in destructor
  2019-05-10  0:23 [PATCH] lib/message-file: close stream in destructor David Bremner
  2019-05-10  1:05 ` Will Dietz
@ 2019-05-10 15:33 ` David Bremner
  1 sibling, 0 replies; 4+ messages in thread
From: David Bremner @ 2019-05-10 15:33 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

> Without this,
>
> $ make time-test OPTIONS=--small
>
> leads to fatal errors from too many open files.
>
> Thanks to st-gourichon-fid for bringing this problem to my attention in IRC.
> ---
>  lib/message-file.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> The regression test could be improved, but I wanted to get a fix out
> and hopefully into master, as this is a problem that seems
> particularly likely to hit new users during their initial notmuch-new.
>
> diff --git a/lib/message-file.c b/lib/message-file.c
> index 50855067..24c5fda4 100644
> --- a/lib/message-file.c
> +++ b/lib/message-file.c
> @@ -46,6 +46,9 @@ _notmuch_message_file_destructor (notmuch_message_file_t *message)
>      if (message->message)
>  	g_object_unref (message->message);
>  
> +    if (message->stream)
> +	g_object_unref (message->stream);
> +
>      return 0;
>  }
>  
> -- 
> 2.20.1

I received several reports that this fixes the problem, so I've pushed
it to master.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-05-10 15:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-10  0:23 [PATCH] lib/message-file: close stream in destructor David Bremner
2019-05-10  1:05 ` Will Dietz
2019-05-10  1:42   ` David Bremner
2019-05-10 15:33 ` David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).