Thread: [Sqlrelay-discussion] Forked listener removes unix socket file (patch available)

Brought to you by: mused

sqlrelay-discussion

[Sqlrelay-discussion] Forked listener removes unix socket file (patch available)

From: Renat S. <sr...@st...> - 2011-11-10 15:40:13

Attachments: sqlrlistener.rm_socket.patch

Hello David,

We've discovered weird problem in sqlrelay. In some circumstances forked 
listener removes unix socket file. It happens in alarm handler, when 
forked listener recieves SIGALRM and calls staticlistener's destructor. 
You've already put some protection for pidfile, shared memory and 
semaphores, but not for "unixport". As a result listener stops getting 
new connections by the local socket, though continues to accept 
connection by tcp.

Please look at the patch attached.


-- 
Renat Sabitov   e-mail: sr...@st...
Stack Soft         jid: sr...@ja...

Re: [Sqlrelay-discussion] Forked listener removes unix socket file (patch available)

From: David M. <dav...@fi...> - 2011-11-10 22:35:39

Ahhh, I see the problem...

I think a cleaner solution might be to remove the setting of pidfile to 
NULL after the fork() and just wrap the removal of the pid and unixport 
files with an "if (!isforkedchild) {" in the destructor.  Then the 
semset, idmemory and cmdl objects would still be freed.

What do you think?

Dave

On 11/10/2011 10:41 AM, Renat Sabitov wrote:
> Hello David,
>
> We've discovered weird problem in sqlrelay. In some circumstances 
> forked listener removes unix socket file. It happens in alarm handler, 
> when forked listener recieves SIGALRM and calls staticlistener's 
> destructor. You've already put some protection for pidfile, shared 
> memory and semaphores, but not for "unixport". As a result listener 
> stops getting new connections by the local socket, though continues to 
> accept connection by tcp.
>
> Please look at the patch attached.
>
>
>
> ------------------------------------------------------------------------------
> RSA(R) Conference 2012
> Save $700 by Nov 18
> Register now
> http://p.sf.net/sfu/rsa-sfdev2dev1
>
> _______________________________________________________
> Unlimited Disk, Data Transfer, PHP/MySQL Domain Hosting
>                http://www.doteasy.com
>
>
> _______________________________________________
> Sqlrelay-discussion mailing list
> Sql...@li...
> https://lists.sourceforge.net/lists/listinfo/sqlrelay-discussion
>
>
> _______________________________________________________
> Unlimited Disk, Data Transfer, PHP/MySQL Domain Hosting
>                http://www.doteasy.com

Re: [Sqlrelay-discussion] Forked listener removes unix socket file (patch available)

From: Renat S. <sr...@st...> - 2011-11-11 05:07:51

Yes, it might be a better solution.

Yet, calling destructors and manipulating semaphores and shared memory 
in signal handler is not recommended at all. The best way to handle a 
signal is to fire some flag in the handler and then check it in the main 
program flow.

11.11.2011 02:39, David Muse пишет:
> Ahhh, I see the problem...
>
> I think a cleaner solution might be to remove the setting of pidfile to
> NULL after the fork() and just wrap the removal of the pid and unixport
> files with an "if (!isforkedchild) {" in the destructor.  Then the
> semset, idmemory and cmdl objects would still be freed.
>
> What do you think?
>
> Dave

-- 
Renat Sabitov   e-mail: sr...@st...
Stack Soft         jid: sr...@ja...

Re: [Sqlrelay-discussion] Forked listener removes unix socket file (patch available)

From: David M. <dav...@fi...> - 2011-11-11 05:46:05

I applied the fix and it's in CVS now.

You're right about signal handlers setting flags and the main app 
testing for them and calling the shutdown code itself.  That would 
definitely eliminate race conditions, like a signal being received 
before isforkedlistener is set but then the shutdown code testing 
isforkedlistener.

I have wondered about how to deal with problems like that before.  I 
can't believe the flag solution never occurred to me.  It seems so 
obvious now.  Feel free to work on that :)

Dave

On 11/11/2011 12:09 AM, Renat Sabitov wrote:
> Yes, it might be a better solution.
>
> Yet, calling destructors and manipulating semaphores and shared memory
> in signal handler is not recommended at all. The best way to handle a
> signal is to fire some flag in the handler and then check it in the main
> program flow.
>
> 11.11.2011 02:39, David Muse пишет:
>> Ahhh, I see the problem...
>>
>> I think a cleaner solution might be to remove the setting of pidfile to
>> NULL after the fork() and just wrap the removal of the pid and unixport
>> files with an "if (!isforkedchild) {" in the destructor.  Then the
>> semset, idmemory and cmdl objects would still be freed.
>>
>> What do you think?
>>
>> Dave

Re: [Sqlrelay-discussion] Forked listener removes unix socket file (patch available)

From: Renat S. <sr...@st...> - 2011-11-14 14:08:50

Attachments: alarm_handling.patch semset_do_not_ignore_interrupts.patch

Hello David,

I've made patches for rudiments and sqlrelay to handle signals by flag. 
Please check it and tell what your thoughts are.

11.11.2011 09:50, David Muse пишет:
> I applied the fix and it's in CVS now.
>
> You're right about signal handlers setting flags and the main app
> testing for them and calling the shutdown code itself.  That would
> definitely eliminate race conditions, like a signal being received
> before isforkedlistener is set but then the shutdown code testing
> isforkedlistener.
>
> I have wondered about how to deal with problems like that before.  I
> can't believe the flag solution never occurred to me.  It seems so
> obvious now.  Feel free to work on that :)
>
> Dave
>
> On 11/11/2011 12:09 AM, Renat Sabitov wrote:
>> Yes, it might be a better solution.
>>
>> Yet, calling destructors and manipulating semaphores and shared memory
>> in signal handler is not recommended at all. The best way to handle a
>> signal is to fire some flag in the handler and then check it in the main
>> program flow.
>>
>> 11.11.2011 02:39, David Muse пишет:
>>> Ahhh, I see the problem...
>>>
>>> I think a cleaner solution might be to remove the setting of pidfile to
>>> NULL after the fork() and just wrap the removal of the pid and unixport
>>> files with an "if (!isforkedchild) {" in the destructor.  Then the
>>> semset, idmemory and cmdl objects would still be freed.
>>>
>>> What do you think?
>>>
>>> Dave
>
>
>
> ------------------------------------------------------------------------------
> RSA(R) Conference 2012
> Save $700 by Nov 18
> Register now
> http://p.sf.net/sfu/rsa-sfdev2dev1
> _______________________________________________
> Sqlrelay-discussion mailing list
> Sql...@li...
> https://lists.sourceforge.net/lists/listinfo/sqlrelay-discussion


-- 
Renat Sabitov   e-mail: sr...@st...
Stack Soft         jid: sr...@ja...

Re: [Sqlrelay-discussion] Forked listener removes unix socket file (patch available)

From: David M. <dav...@fi...> - 2011-11-14 17:29:40

Wow, that was fast!  Thanks, I'll check them out and let you know soon.

Dave

On 11/14/2011 09:08 AM, Renat Sabitov wrote:
> Hello David,
>
> I've made patches for rudiments and sqlrelay to handle signals by 
> flag. Please check it and tell what your thoughts are.
>
> 11.11.2011 09:50, David Muse пишет:
>> I applied the fix and it's in CVS now.
>>
>> You're right about signal handlers setting flags and the main app
>> testing for them and calling the shutdown code itself.  That would
>> definitely eliminate race conditions, like a signal being received
>> before isforkedlistener is set but then the shutdown code testing
>> isforkedlistener.
>>
>> I have wondered about how to deal with problems like that before.  I
>> can't believe the flag solution never occurred to me.  It seems so
>> obvious now.  Feel free to work on that :)
>>
>> Dave
>>
>> On 11/11/2011 12:09 AM, Renat Sabitov wrote:
>>> Yes, it might be a better solution.
>>>
>>> Yet, calling destructors and manipulating semaphores and shared memory
>>> in signal handler is not recommended at all. The best way to handle a
>>> signal is to fire some flag in the handler and then check it in the 
>>> main
>>> program flow.
>>>
>>> 11.11.2011 02:39, David Muse пишет:
>>>> Ahhh, I see the problem...
>>>>
>>>> I think a cleaner solution might be to remove the setting of 
>>>> pidfile to
>>>> NULL after the fork() and just wrap the removal of the pid and 
>>>> unixport
>>>> files with an "if (!isforkedchild) {" in the destructor.  Then the
>>>> semset, idmemory and cmdl objects would still be freed.
>>>>
>>>> What do you think?
>>>>
>>>> Dave
>>
>>
>>
>> ------------------------------------------------------------------------------ 
>>
>> RSA(R) Conference 2012
>> Save $700 by Nov 18
>> Register now
>> http://p.sf.net/sfu/rsa-sfdev2dev1
>> _______________________________________________
>> Sqlrelay-discussion mailing list
>> Sql...@li...
>> https://lists.sourceforge.net/lists/listinfo/sqlrelay-discussion
>
>
>
> ------------------------------------------------------------------------------
> RSA(R) Conference 2012
> Save $700 by Nov 18
> Register now
> http://p.sf.net/sfu/rsa-sfdev2dev1
>
> _______________________________________________________
> Unlimited Disk, Data Transfer, PHP/MySQL Domain Hosting
>                http://www.doteasy.com
>
>
> _______________________________________________
> Sqlrelay-discussion mailing list
> Sql...@li...
> https://lists.sourceforge.net/lists/listinfo/sqlrelay-discussion
>
>
> _______________________________________________________
> Unlimited Disk, Data Transfer, PHP/MySQL Domain Hosting
>                http://www.doteasy.com