On Nov 23, 2016, at 4:13 AM, Christian Seiler <christian.is...@gmail.com> wrote:
> 
> Hi,
> 
> On 11/22/2016 11:43 PM, leeman.dun...@gmail.com wrote:
>> static void catch_signal(int signo)
>> [...]
>>      switch (signo) {
>>      case SIGTERM:
>> -            iscsid_shutdown();
>> +            iscsid_shutdown_and_cleanup();
>>              exit(0);
>>              break;
> 
> I think this is not a good idea because most functions called
> here will not be async signal safe, and you are calling them
> from a signal handler. Beforehand this was not a problem since
> iscsid_shutdown() is just a kill of the process group.

The signal is ignored while in the signal handler, and I expect
not to get a SIGTERM storm. But perhaps you are right.

> 
> This might lead to very ugly race condition and deadlocks, and
> the race conditions could cause crashes or even arbitrary
> memory to be overwritten in the worst case. (Think code that
> is in the middle of updating a data structure, and the signal
> comes at a time when the structure is in an inconsistent
> state.)
> 
> Creating async-signal-safe data structures is not impossible
> (as long as atomic operations are available), but very hard to
> do properly, and even harder when shutdown of these data
> structures in a signal handler is to be supported. Writing
> thread-safe code is much, much easier, for example.
> 
> The recommended way of performing complicated actions in
> response to signals in daemons is to have the signal handler
> set a flag (either using atomic operations or with the good
> old "volatile") and have the regular event loop always check
> to see if the flag was changed, and if so initiate a
> shutdown.

I will resubmit the patch series with the 2nd patch updated.

I see now (looking at the code), that I can just set the “exit
the event loop” flag when SIGTERM is received. And it has
the added benefit of being idempotent, so I don’t need to
guard against receiving multiple signals.

> 
> Regards,
> Christian
> 

-- 
Lee-Man
"Reincarnated as love poetry." -- Big Head Todd

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

Reply via email to