gluster-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Gluster-devel] glusterd crashes when synctask is used in conjunctio


From: ಅಮರ್ ತುಂಬಳ್ಳಿ
Subject: Re: [Gluster-devel] glusterd crashes when synctask is used in conjunction with inner functions.
Date: Tue, 9 Oct 2012 08:33:39 +0530



On Mon, Oct 8, 2012 at 6:10 PM, Jeff Darcy <address@hidden> wrote:
On October 8, 2012 7:02:34 AM Krishnan Parthasarathi <address@hidden> 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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]