Re: [PATCH 7/6] Enable threaded async procedures whenever pthreads is available

[Johannes Sixt <j6t@kdbg.org>]

On Dienstag, 23. März 2010, Fredrik Kuivinen wrote:
> On Wed, Mar 17, 2010 at 22:28, Johannes Sixt <j6t@kdbg.org> wrote:
> > ----------
> > convert.c:filter_buffer()
> ...
> Maybe I'm missing something but, isn't it possible that xrealloc is
> called simultaneously from the two threads if GIT_TRACE is set?
>
> Immediately after start_async the parent calls strbuf_read. We then
> get the call chain
> strbuf_read -> strbuf_grow -> ALLOG_GROW -> xrealloc, so xrealloc is
> called before we read any data in the parent.
>
> In the child we have start_command -> trace_argv_printf -> strbuf_grow ->
> ...

Outch! You are right. It seems I missed the call of strbuf_grow before the 
loop in strbuf_read.

OK, this means that convert.c is not safe if (and only if) GIT_TRACE is 
set. :-(

> That xmalloc and xrealloc aren't thread-safe feels a bit fragile.
> Maybe we should try to fix that.

The point of this assessment was to find out whether this is necessary (and 
whether something else that is not thread-safe is used).

> > ----------
> > upload_pack:create_pack_file(): 
> ...
> sha1_to_hex is also called by the parent and the current
> implementation of that function is not thread-safe. sha1_to_hex is
> also called by some paths in the revision machinery, but I don't know
> if it will ever be called in this particular case.

sha1_to_hex is only called by the parent when the async procedure is not used.

-- Hannes