Yes, I think it would be safer to have it that way. :-) regards, Anders Widell
2013-10-10 11:09, Anders Björnerstedt skrev: > The conclusion is that this should be fixed (coverting the global inline > static data to > be either not global or to be only global pointer data) in IMMND at some > point in time. > > > AndersBj > > -----Original Message----- > From: Anders Widell > Sent: den 10 oktober 2013 11:04 > To: Anders Björnerstedt; Hans Feldt; mathi.naic...@oracle.com > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [devel] [PATCH 1 of 1] opensaf: change daemon_exit to call > exit() [#581] > > Yes, but even if you protect the global data with e.g. a mutex, you can still > get unexpected problems when you call exit(). exit() will destroy all your > global data, and it does not know about any mutex you may have. > The data will be destroyed even if your mutex is locked. This is why it is so > dangerous. > > In the case of IMM, if only the main thread accesses the global data then it > is safe to call exit() from the main thread. Calling exit() from the MDS > thread, on the other hand, can cause problems. > > The problem goes away if we have a global pointer rather than a global > object. This is because exit() will only destroy the global pointer itself, > not the object it is pointing to (unless it is a shared_ptr or similar, of > course). > > regards, > Anders Widell > > 2013-10-10 10:50, Anders Björnerstedt skrev: >> And if only the main thread does exit() >> >> Using multiple threads to access shared data means you have to syncronize .. >> Nothing new in that. >> In any case, the static c++ data (STL objects) that are used in IMMND >> are *only* accessed by the main thread. The only potential risk I see is if >> for example the MDS thread does an exit(). >> >> /AndersBj >> >> -----Original Message----- >> From: Anders Widell >> Sent: den 10 oktober 2013 10:32 >> To: Anders Björnerstedt; Hans Feldt; mathi.naic...@oracle.com >> Cc: opensaf-devel@lists.sourceforge.net >> Subject: Re: [devel] [PATCH 1 of 1] opensaf: change daemon_exit to >> call exit() [#581] >> >> Yes, I was referring to the episode with exit() in the signal handler. >> Calling exit() in the main thread can cause the same problem if the global >> data is accessed by other threads, and we don't terminate those threads >> before calling exit(). If only the main thread ever accesses the data, then >> it ought to be safe. >> >> regards, >> Anders Widell >> >> 2013-10-09 19:49, Anders Björnerstedt skrev: >>> Or perhaps not the same kind of crash. >>> But I dont think you are saying that we should alter the imm >>> implementation to allow exit in the signal handler?! >>> >>> Besides the main thread, the immnd process has some hidden threads, >>> for example in the MDS library linked into the process. These other >>> thread definitely do not access these structures. But they may invoke >>> exit() or abort(). >>> >>> But I have never seen any interference caused by that, such as >>> strange core dumps while navigating the statically allocated c++ data. >>> >>> Only the exit donw by the signal handler has caused that kind of problem. >>> >>> /AndersBj >>> >>> >>> -----Original Message----- >>> From: Anders Björnerstedt [mailto:anders.bjornerst...@ericsson.com] >>> Sent: den 9 oktober 2013 19:33 >>> To: Anders Widell; Hans Feldt; mathi.naic...@oracle.com >>> Cc: opensaf-devel@lists.sourceforge.net >>> Subject: Re: [devel] [PATCH 1 of 1] opensaf: change daemon_exit to >>> call exit() [#581] >>> >>> And no they are not acccessed by any other thread than the main thread. >>> >>> The problem we had, if I remember correctly, was an exit() had been >>> inserted in the signal handler. This "pulled the rug out from under" the >>> main thread of course. >>> But that problem was quickly fixed once we realized what it was doing. >>> And I dont see that it was related to statically allocated objects. >>> Even if we had dynamically allocated objects with static pointers, the same >>> kind Of crash would have occurred. >>> >>> /AndersBj >>> >>> -----Original Message----- >>> From: Anders Björnerstedt >>> Sent: den 9 oktober 2013 19:23 >>> To: Anders Widell; Hans Feldt; mathi.naic...@oracle.com >>> Cc: opensaf-devel@lists.sourceforge.net >>> Subject: RE: [devel] [PATCH 1 of 1] opensaf: change daemon_exit to >>> call exit() [#581] >>> >>> Played with exit? >>> Are you referring to the episode with illegal stuff in the signal handler ? >>> >>> /AndersBj >>> >>> -----Original Message----- >>> From: Anders Widell >>> Sent: den 9 oktober 2013 16:28 >>> To: Anders Björnerstedt; Hans Feldt; mathi.naic...@oracle.com >>> Cc: opensaf-devel@lists.sourceforge.net >>> Subject: Re: [devel] [PATCH 1 of 1] opensaf: change daemon_exit to >>> call exit() [#581] >>> >>> Wasn't it IMM that got into trouble last time we played with exit() >>> :-) >>> >>> These objects will be destroyed when we call exit(). Are they accessed by >>> any thread besides the main thread? >>> >>> Anyway, I suppose it wouldn't be too hard to change these variables into >>> pointers, and initialize them the first thing we do in main() ? >>> >>> regards, >>> Anders Widell >>> >>> 2013-10-09 16:12, Anders Björnerstedt skrev: >>>> The imm uses static global (non-pointer) variables for a number of C++ STL >>>> objects. >>>> Maps, Sets, Vectors etc. >>>> >>>> I have never seen any problems with this. >>>> >>>> /AndersBj >>>> >>>> -----Original Message----- >>>> From: Anders Widell [mailto:anders.wid...@ericsson.com] >>>> Sent: den 9 oktober 2013 15:52 >>>> To: Hans Feldt; mathi.naic...@oracle.com >>>> Cc: opensaf-devel@lists.sourceforge.net >>>> Subject: Re: [devel] [PATCH 1 of 1] opensaf: change daemon_exit to >>>> call exit() [#581] >>>> >>>> Ack from me. >>>> >>>> I guess we should also go through the code and make sure we follow this >>>> rule from the Google C++ Style guide: >>>> >>>> Static and Global Variables >>>> >>>> Static or global variables of class type are forbidden: they cause >>>> hard-to-find bugs due to indeterminate order of construction and >>>> destruction. However, such variables are allowed if they are constexpr: >>>> they have no dynamic initialization or destruction. >>>> >>>> regards, >>>> Anders Widell >>>> >>>> 2013-10-03 14:54, Hans Feldt skrev: >>>>> osaf/libs/core/common/daemon.c | 9 ++++++--- >>>>> 1 files changed, 6 insertions(+), 3 deletions(-) >>>>> >>>>> >>>>> By calling exit() instead of _Exit() registered exit functions are >>>>> called. This enabled for example flushing of gcov data. >>>>> >>>>> diff --git a/osaf/libs/core/common/daemon.c >>>>> b/osaf/libs/core/common/daemon.c >>>>> --- a/osaf/libs/core/common/daemon.c >>>>> +++ b/osaf/libs/core/common/daemon.c >>>>> @@ -355,13 +355,16 @@ static void sigterm_handler(int sig) >>>>> } >>>>> >>>>> /** >>>>> - * Exit process with a standard syslog message >>>>> - * To be called after the service has cleaned up per service >>>>> specific things >>>>> + * Exit calling process with exit(0) using a standard syslog message. >>>>> + * This function should be called from the main thread of a server >>>>> + process in >>>>> + * a "safe" context for calling exit(). Any service specific thing >>>>> + should be >>>>> + * cleaned up before calling this function. By calling exit(), >>>>> + registered exit >>>>> + * functions are called before the process is terminated. >>>>> */ >>>>> void daemon_exit(void) >>>>> { >>>>> syslog(LOG_NOTICE, "exiting on signal %d", SIGTERM); >>>>> - _Exit(EXIT_SUCCESS); >>>>> + exit(0); >>>>> } >>>>> >>>>> /** >>>> -------------------------------------------------------------------- >>>> - >>>> - >>>> -------- October Webinars: Code for Performance Free Intel webinars >>>> can help you accelerate application performance. >>>> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the >>>> most from the latest Intel processors and coprocessors. See >>>> abstracts and register > >>>> http://pubads.g.doubleclick.net/gampad/clk?id=60134071&iu=/4140/ostg. >>>> c lktrk _______________________________________________ >>>> Opensaf-devel mailing list >>>> Opensaf-devel@lists.sourceforge.net >>>> https://lists.sourceforge.net/lists/listinfo/opensaf-devel >>> --------------------------------------------------------------------- >>> - >>> -------- October Webinars: Code for Performance Free Intel webinars >>> can help you accelerate application performance. >>> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the >>> most from the latest Intel processors and coprocessors. See abstracts >>> and register > >>> http://pubads.g.doubleclick.net/gampad/clk?id=60134071&iu=/4140/ostg. >>> c lktrk _______________________________________________ >>> Opensaf-devel mailing list >>> Opensaf-devel@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/opensaf-devel > ------------------------------------------------------------------------------ October Webinars: Code for Performance Free Intel webinars can help you accelerate application performance. Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from the latest Intel processors and coprocessors. See abstracts and register > http://pubads.g.doubleclick.net/gampad/clk?id=60134071&iu=/4140/ostg.clktrk _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
