Re: [Log4cplus-devel] Socket close hangs

[Václav Z. <vha...@gm...>]

On 09/20/2013 05:07 PM, palladin wrote:
> On 09/19/2013 01:15 AM, Václav Zeman wrote:
>> On 09/18/2013 09:51 AM, palladin wrote:
>>> Dear log4cplus devels,
>>> log4cplus 1.1.1-RC4
>>> <https://sourceforge.net/projects/log4cplus/files/log4cplus-stable/1.1.1/>
>>> Linux version 2.6.32-279.22.1.37.0.el6.x86_64 (abuild@noriko) (gcc
>>> version 4.4.6 20120305 (Red Hat 4.4.6-4) (GCC) )
>>>
>>> My application has hung after it have used log4cplus::helpers::Socket
>>> from  log4cplus::helpers::ServerSocket::accept,e.g.
>>> log4cplus::helpers::Socket clientSocket = mServerSocket.accept();
>>> clientSocket was put into log4cplus::thread::AbstractThread worker.
>>> Client socket was used in AbstractThread like Socket::read - this is
>>> blocking call and the root of issue.
>>>
>>> Then my application worked for some time  and at the end I did
>>> clientSocket.close() from main thread.
>>> Due to the fact that log4cplus::helpers::Socket::read is blocking call -
>>> my application has hung on AbstractThread::join call.
>>>
>>> I would like to suggest a fiew fixes to this issue:
>>> 1) add non-blocking stuff to log4cplus::helpers::Socket - select or poll.
>>> 2) add to log4cplus::helpers::Socket interface system call shutdown( man
>>> 2 shutdown ).
>> Closing a socket from one thread when the socket is blocked on any
>> operation in another thread is AFAIK very unportable.
>>
>> This could be solved in a way similar to how I have done
>> ServerSocket::accept(), which is indeed using poll() to wait for either
>> an incoming connection or some data arriving to a special interrupt pipe.
>>
>> Also, these helpers are there to help log4cplus. They are not there to
>> be used as public networking framework/API, so fixing this has a low
>> priority for me right now. OTOH, I am open to accepting reasonable patches.
>>
> 
> Dear Václav,
> I prefer your solution in ServerSocket interruption. So could you please
> take a look on provided patch?
> 
> Sincerely ,
> Taras
TBH, I do not like the patch that much. I see several problems:

- It does not take Windows into consideration.
- the MAX() macro instead of std::max function
- use of select() instead of poll()
- I would rather like to see the interruptibility implemented in
existing sockets classes rather than in new kind of socket
InterruptableSocket.
- It lacks error handling at few places.

-- 
VZ