[Gluster-devel] glusterd crashes when synctask is used in conjunction with inner functions.

[Amar (ಅಮರ್ ತುಂಬಳ್ಳಿ)]

On Mon, Oct 8, 2012 at 6:10 PM, Jeff Darcy <jdarcy at redhat.com> wrote:

> On October 8, 2012 7:02:34 AM Krishnan Parthasarathi <kparthas at redhat.com>
> wrote:
>
>> I have been rewriting some of the volume operations (like volume-start,
>> volume-add-brick, volume-remove-brick) using synctask library (aka
>> syncops).
>> This change has the following immediate benefits,
>>
>> - volume-start would return success/failure depending on the
>> success/failure of
>>   brick process(es) spawned.
>> - would make glusterd's epoll thread 'more' available.
>>
>> </context>
>>
>> While I was making the changes in http://review.gluster.com/3969**, I
>> noticed that
>> whenever the code executing on a synctask called into dict_foreach, which
>> was supplied
>> a function ptr, defined as an inner function, glusterd crashed. When I
>> rewrote
>> inner function as a static function, glusterd wouldn't crash.
>>
>> Has anyone seen or can explain (or give possible leads to analyse) this
>> behaviour?
>> FWIW, inner functions are only available as part of GNU extensions to C.
>> So, I
>> assumed it is not such a bad thing to move the inner functions 'out', in
>> my patch.
>>
>
> Ugh.  I noticed this pattern while I was looking at some AFR stuff
> recently.  I thought it was rather clever, and pondered a bit about how it
> might be implemented in gcc/libc.  Apparently it was a bit too clever, and
> the implementation leaves something to be desired.  An inner function might
> be more elegant than defining private structures to pass through our own
> context pointer, but in the interest of both portability and not having to
> debug compiler code I think changing these to work the "old fashioned" way
> would actually be a good thing.
>
>
>
KP/Jeff,

It was me who introduced inner functions with patch
http://review.gluster.org/3829.

The reason was to keep the changes as minimal as possible within that
particular patch. I have no objections to remove the inner functions
completely from the codebase.

But it was good exercise to understand the reason for crash though :-)

Regards,
Amar
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://supercolony.gluster.org/pipermail/gluster-devel/attachments/20121009/40e1df0f/attachment-0001.html>