zsh-workers
 help / color / mirror / code / Atom feed
From: Bart Schaefer <schaefer@brasslantern.com>
To: Kamil Dudka <kdudka@redhat.com>, zsh-workers@zsh.org
Subject: Re: deadlock in free() called from a signal handler
Date: Thu, 19 Feb 2015 09:23:29 -0800	[thread overview]
Message-ID: <150219092329.ZM17912@torch.brasslantern.com> (raw)
In-Reply-To: <37490085.zXPQGCoLTl@kdudka.brq.redhat.com>

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.


  reply	other threads:[~2015-02-19 17:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-19 11:06 Kamil Dudka
2015-02-19 17:23 ` Bart Schaefer [this message]
2015-02-21  2:28   ` PATCH " Bart Schaefer
2015-02-23 14:15     ` Kamil Dudka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=150219092329.ZM17912@torch.brasslantern.com \
    --to=schaefer@brasslantern.com \
    --cc=kdudka@redhat.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

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 NNTP newsgroup(s).