Re: deadlock in free() called from a signal handler

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Feb 19, 12:06pm, Kamil Dudka wrote:
} Subject: deadlock in free() called from a signal handler
}
} We have a bug report about deadlock in zsh due to a call to free() from
} a signal handler.

In every case I can remember so far, when this happens it means that we
ought to be using the signal queuing macros at a scope outside the call
to the malloc library.

} I have discovered a similar issue here on the list:
} 
} http://www.zsh.org/mla/workers/2014/msg01402.html
} 
} However, the above comment does not sound correct to me.  zfree() contains
} calls to do signal queueing, only if zsh is compiled with ZSH_MEM, which
} is not the default configuration.

Yes, I believe this eventually got noted in another thread (or another
branch of that thread) because we later came upon a couple of other
cases where signal queuing was needed around some global state updates.

} Is this on purpose?

It's not by accident, but it's been that way forever, so the reasoning
is lost in time.  I believe that at one point there was no zfree() and
ZSH_MEM simply redefined free() directly, so the other branch may at
one time have been empty and was only filled in with the minimal code
when the zfree symbol was introduced.

} Would it make sense to surround also the plain free() wrapper by the
} signal queueing macros? I would be happy to provide a patch...

I was going to say no, it's not, because we don't wrap malloc() calls
similarly ... but on a quick examination there is only one direct call
to malloc() left in the source anywhere outside of mem.c, and mem.c
does wrap queue_signals() around all calls to malloc().

So ... per my very first remark above, it's probably worth examining the
context in execcmd() to see if signal queuing is appropriate there, or
if we just need it in setunderscore().  But it's may also make sense to
replace that one last malloc() and put queuing in zfree()/zsfree() too.