From: Tomi Ollila <tomi.ollila@iki.fi>
To: David Bremner <david@tethera.net>, Kazuo Teramoto <kaz.rag@gmail.com>,
	notmuch@notmuchmail.org
Subject: Re: [PATCH 2/2] lib: call g_mime_init from notmuch_database_open
In-Reply-To: <87hb0h5tps.fsf@zancas.localnet>
References: <877h1e6r9d.fsf@zancas.localnet>
	<1325282290-29565-1-git-send-email-kaz.rag@gmail.com>
	<1325282290-29565-3-git-send-email-kaz.rag@gmail.com>
	<87hb0h5tps.fsf@zancas.localnet>
User-Agent: Notmuch/0.10.2+157~g442d405 (http://notmuchmail.org) Emacs/23.3.1
	(i686-pc-linux-gnu)
Date: Mon, 02 Jan 2012 14:56:13 +0200
Message-ID: <yf6obumz2j6.fsf@taco2.nixu.fi>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Fri, 30 Dec 2011 23:02:39 -0400, David Bremner <david@tethera.net> wrote:
> On Fri, 30 Dec 2011 19:58:10 -0200, Kazuo Teramoto <kaz.rag@gmail.com> wrote:
> > We need to call g_mime_init to correct initialize the structures needed
> > by gmime before using it.
> > ---
> >  lib/database.cc |    5 +++++
 > >  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> I can confirm this patch (alone) fixes the segfault for me. and doesn't
> cause in test failures when applied against the release branch.  I'm
> considering pushing it into the 0.11 release. Any comments on that?

Do it. The current implementation does not break anything (in our case
as we don't do g_mime_shutdown() (0, 1 or 2 times)... The usage needs to
be "fixed" soon after...

> d

.. my suggestions how to fix this:

1) just call g_mime_init(0) without checking whether it is initialized
   already. As it't return type is 'void' it can be expected to keep some
   global storage (and reference count...). If it returned something else
   it either returns common (singleton) instance (and keeps reference count)
   or returns new instance each time called.... Never call g_mime_shutdown();
   just let operating system free resources when program exits. The first
   lines in g_mime_shutdown():

        if (--initialized)
                return;

   looks a bit suspicious -- what if application happens to call
   g_mime_shutdown() too often and then trying to call g_mime_init() again...

2) create function

  void
  notmuch_g_type_init (void)
  {
        static int initialized;
        if (initialized++)
        	return;
        g_mime_init(0)
        atexit (g_mime_shutdown);
  }

  and use that in place of g_mime_init() always (perhaps use macro trickery
  to disallow g_mime_init (and g_mime_shutdown). 

  Note that although atexit() makes pretty sure that in normal exit 
  g_mime_shutdown() is called due to various reasons that normal exit
  case is not reached (dies by signal, usage of _exit, execve(2)
  hardware failure, lost power etc.)(*). Therefore, IMO, I'd leave this
  (particular) cleanup to be done by the operating system -- less, 
  simpler application code.


Tomi

(*) I recall reading an article about 'design for failure' or 'expect
    failure' like 5 years ago but cannot find it anymore. The point on that
    article was that software can fail in any point (specially, compare
    man 2 rename  and then around lines 580-610 in
    http://git.busybox.net/busybox/tree/sysklogd/syslogd.c ). Since then
    I've tried to prioritize in finalize things that matter early and leave
    operating system to do other cleanpus for me (why do something yourself
    that machine can do for you). Did you know that doing full windows
    reboot is faster if the machine is just unplugged from power source
    for a second and then restarted instead of doing "clean" reboot cycle.
    Linux boots so fast I don't know if I'd notice the difference ;)