[tarantool-patches] Re: [PATCH 01/10] box: zap atfork callback

[Konstantin Osipov]

* Konstantin Osipov <kostja at tarantool.org> [19/05/18 21:41]:
> * Vladimir Davydov <vdavydov.dev at gmail.com> [19/05/17 17:54]:
> > box_atfork calls wal_atfork which in turn calls xlog_atfork for the wal
> > and vylog files. A comment to xlog_atfork says that it's necessary to
> > prevent atexit handlers in a child from closing xlog files again, but we
> > don't use atexit for that anymore. A comment to box_atfork says that
> > box.coredump forks to write a core, but there's no box.coredump anymore.
> > There's also a comment mentioning box.cfg.background, but when we fork
> > that early there's no xlog file open.
> > 
> > To sum it up, atfork looks like a piece of legacy code. Let's get rid of
> > it now so as not to bother patching it later.
> 
> If anyone adds a fork any time in the future, xlogs will break
> silently, and it would be hard to catch in a test. 
> 
> If you wish to remove the dead code, please keep the atfork, but panic in it.

As discussed in the channel, you can't panic in atfork handler
since there is an upcoming popen() patch. This all still looks
very fragile.
I read from vfork() man page that NPTL doesn't call
pthread_atfork() handlers in vfork,
while older implementation may. What is the behaviour on freebsd? 
If both systems don't invoke the handlers, I would keep the
panic() call, otherwise remove.

-- 
Konstantin Osipov, Moscow, Russia