External Semaphores leaking in Socket
I took some time to analyze my current problem with external semaphores. I was just reluctant to raise the limit in my image because I want the problem solved. I logged the management of external semaphores and discovered that the table fills if a connection times out. The problem turns out to be in Socket>>#connectTo: hostAddress port: port waitForConnectionFor: timeout "Initiate a connection to the given port at the given host address. Waits until the connection is established or time outs." self connectNonBlockingTo: hostAddress port: port. self waitForConnectionFor: timeout ifTimedOut: [ConnectionTimedOut signal: 'Cannot connect to ' , (NetNameResolver stringFromAddress: hostAddress) , ':' , port asString] When a socket is created three external semaphores are registered in the ExternalSemaphoreTable. If a connection times out the exception is thrown but the Socket still has his resources attached. So e.g. in SocketStream class>>#openConnectionToHost: hostIP port: portNumber timeout: timeout | socket | socket := Socket new. socket connectTo: hostIP port: portNumber waitForConnectionFor: timeout. ^self on: socket it holds locally a socket (with semaphores registered) but on exception time the reference to the socket gets lost and the semaphores stay registered. The only way to unregister is on finalization time but I think it should work better. So I would add a destroy before the exception is raised. Socket>>#connectTo: hostAddress port: port waitForConnectionFor: timeout "Initiate a connection to the given port at the given host address. Waits until the connection is established or time outs." self connectNonBlockingTo: hostAddress port: port. self waitForConnectionFor: timeout ifTimedOut: [ self destroy. ConnectionTimedOut signal: 'Cannot connect to ' , (NetNameResolver stringFromAddress: hostAddress) , ':' , port asString] I opened a ticket at https://pharo.fogbugz.com/f/cases/11797 but I'm not sure how I am supposed to provide fixes made against a pharo2.0 image. Probably I should fix this againt 3.0 but then I'm still a 2.0 user :) Norbert
On Oct 6, 2013, at 12:51 PM, Norbert Hartl <norbert@hartl.name> wrote:
I opened a ticket at https://pharo.fogbugz.com/f/cases/11797 but I'm not sure how I am supposed to provide fixes made against a pharo2.0 image. Probably I should fix this againt 3.0 but then I'm still a 2.0 user :)
The best is to have two issues: one for 2.0 and one for 3.0. Then we can first commit it to Pharo3, test some days and then do the pharo2 version (you need to do two slices, one for 2.0 in the 2.0 inbox and one for 3.0) Marcus
On Oct 6, 2013, at 12:51 , Norbert Hartl <norbert@hartl.name> wrote:
I took some time to analyze my current problem with external semaphores. I was just reluctant to raise the limit in my image because I want the problem solved. I logged the management of external semaphores and discovered that the table fills if a connection times out.
The problem turns out to be in
Socket>>#connectTo: hostAddress port: port waitForConnectionFor: timeout "Initiate a connection to the given port at the given host address. Waits until the connection is established or time outs." self connectNonBlockingTo: hostAddress port: port. self waitForConnectionFor: timeout ifTimedOut: [ConnectionTimedOut signal: 'Cannot connect to ' , (NetNameResolver stringFromAddress: hostAddress) , ':' , port asString]
When a socket is created three external semaphores are registered in the ExternalSemaphoreTable. If a connection times out the exception is thrown but the Socket still has his resources attached.
So e.g. in
SocketStream class>>#openConnectionToHost: hostIP port: portNumber timeout: timeout | socket | socket := Socket new. socket connectTo: hostIP port: portNumber waitForConnectionFor: timeout. ^self on: socket
it holds locally a socket (with semaphores registered) but on exception time the reference to the socket gets lost and the semaphores stay registered. The only way to unregister is on finalization time but I think it should work better. So I would add a destroy before the exception is raised.
Socket>>#connectTo: hostAddress port: port waitForConnectionFor: timeout "Initiate a connection to the given port at the given host address. Waits until the connection is established or time outs." self connectNonBlockingTo: hostAddress port: port. self waitForConnectionFor: timeout ifTimedOut: [ self destroy. ConnectionTimedOut signal: 'Cannot connect to ' , (NetNameResolver stringFromAddress: hostAddress) , ':' , port asString]
I opened a ticket at https://pharo.fogbugz.com/f/cases/11797 but I'm not sure how I am supposed to provide fixes made against a pharo2.0 image. Probably I should fix this againt 3.0 but then I'm still a 2.0 user :)
Norbert Destroying the socket manually there would only be an optimization, right? It's still cleaned eventually by a finalization action? Finalization all of a sudden stopping to work is an unlikely culprit of running out of external object space, load above what the size can handle, or actual leak(s) are much more probable.
Thankfully, there are a limited number of places leaks could occur, for Socket you have improper uses of acceptFrom:/initialize: (multiple sends to the same socket). In a stock image, there's an obscure way that might occur afaict, if class methods acceptFrom:/newXXX are called, and the primitive calls in the new instance's acceptFrom:/initialize: returns a handle with status InvalidSocket instead of nil. Then, the external objects will not be unregistered before repeatWithGCIf: clears them by calling them again. (No idea if this can ever really happen though...) To flush out if such leaks are the cause, it might be useful to adjust initialize:/acceptFrom: with helpful logging/errors, as per https://pharo.fogbugz.com/f/cases/5465/Prevent-accidental-semaphore-leaks-wh... There's a few other users of registerExternalObject:, - InputEventFetcher >> #startUp (Used to leak when saving but not quitting due to missing symmetric unregister in #shutDown, should be fixed in 2.0 ) - AsynchFile (none uses this anyways right, so didn't bother checking) - NetNameResolver >> initializeNetwork - (This potentially leaked if network could ever be in an uninitialized state 2x in a session, but should be fixed in 2.0) As long as both the above are fixed in your image, checking if there are non-stock users of registerExternalObject:/initialize:/acceptFrom: is probably the best bet to find trouble spots without waiting for another crash. Cheers, Henry
Am 07.10.2013 um 14:16 schrieb Henrik Johansen <henrik.s.johansen@veloxit.no>:
On Oct 6, 2013, at 12:51 , Norbert Hartl <norbert@hartl.name> wrote:
I took some time to analyze my current problem with external semaphores. I was just reluctant to raise the limit in my image because I want the problem solved. I logged the management of external semaphores and discovered that the table fills if a connection times out.
The problem turns out to be in
Socket>>#connectTo: hostAddress port: port waitForConnectionFor: timeout "Initiate a connection to the given port at the given host address. Waits until the connection is established or time outs." self connectNonBlockingTo: hostAddress port: port. self waitForConnectionFor: timeout ifTimedOut: [ConnectionTimedOut signal: 'Cannot connect to ' , (NetNameResolver stringFromAddress: hostAddress) , ':' , port asString]
When a socket is created three external semaphores are registered in the ExternalSemaphoreTable. If a connection times out the exception is thrown but the Socket still has his resources attached.
So e.g. in
SocketStream class>>#openConnectionToHost: hostIP port: portNumber timeout: timeout | socket | socket := Socket new. socket connectTo: hostIP port: portNumber waitForConnectionFor: timeout. ^self on: socket
it holds locally a socket (with semaphores registered) but on exception time the reference to the socket gets lost and the semaphores stay registered. The only way to unregister is on finalization time but I think it should work better. So I would add a destroy before the exception is raised.
Socket>>#connectTo: hostAddress port: port waitForConnectionFor: timeout "Initiate a connection to the given port at the given host address. Waits until the connection is established or time outs." self connectNonBlockingTo: hostAddress port: port. self waitForConnectionFor: timeout ifTimedOut: [ self destroy. ConnectionTimedOut signal: 'Cannot connect to ' , (NetNameResolver stringFromAddress: hostAddress) , ':' , port asString]
I opened a ticket at https://pharo.fogbugz.com/f/cases/11797 but I'm not sure how I am supposed to provide fixes made against a pharo2.0 image. Probably I should fix this againt 3.0 but then I'm still a 2.0 user :)
Norbert Destroying the socket manually there would only be an optimization, right?
Yes. Even if it is not necessary I think freeing resources at the earliest possible time is a good thing. The problem with my change is that I have this gut feeling it is not a good one. At least I didn't see my image hang with this change applied.
It's still cleaned eventually by a finalization action?
Yes, that is what makes me wonder. Why do you say "eventually"? What would be the conditions for finalize not being called on an object at garbage collection time? I log some details and print the external objects size into a lock file. The time of the last hangs the last printed lines where about an external objects size between 230 and 240. I had one test where I interacted with the image and did a manual garbage collect but the external object size was much higher than expected. This is all not very detailed but you get confused very easily hunting problems that are hard to reproduce. So I don't have a glue what it is. Need more time to look into it. But I'm running out of ideas what it could be. I'm not even sure it is the external semaphores directly. Norbert
Finalization all of a sudden stopping to work is an unlikely culprit of running out of external object space, load above what the size can handle, or actual leak(s) are much more probable.
Thankfully, there are a limited number of places leaks could occur, for Socket you have improper uses of acceptFrom:/initialize: (multiple sends to the same socket).
In a stock image, there's an obscure way that might occur afaict, if class methods acceptFrom:/newXXX are called, and the primitive calls in the new instance's acceptFrom:/initialize: returns a handle with status InvalidSocket instead of nil. Then, the external objects will not be unregistered before repeatWithGCIf: clears them by calling them again. (No idea if this can ever really happen though...)
To flush out if such leaks are the cause, it might be useful to adjust initialize:/acceptFrom: with helpful logging/errors, as per https://pharo.fogbugz.com/f/cases/5465/Prevent-accidental-semaphore-leaks-wh...
There's a few other users of registerExternalObject:, - InputEventFetcher >> #startUp (Used to leak when saving but not quitting due to missing symmetric unregister in #shutDown, should be fixed in 2.0 ) - AsynchFile (none uses this anyways right, so didn't bother checking) - NetNameResolver >> initializeNetwork - (This potentially leaked if network could ever be in an uninitialized state 2x in a session, but should be fixed in 2.0)
As long as both the above are fixed in your image, checking if there are non-stock users of registerExternalObject:/initialize:/acceptFrom: is probably the best bet to find trouble spots without waiting for another crash.
Cheers, Henry
Hi Norbert, I come a bit late in the discussion, but here are my ideas. First, I think you did a great job chasing this problem. What you found and your solution are quite useful I think. I am wondering, thinking about some other issues. Does the problem only occur in a ConnectionTimedOut during #connectTo:port: or in any situation when either ConnectionTimedOut or ConnectionClosed are signalled (so during regular reading and/or writing) ? Resource cleanup during exceptional situation is notoriously difficult. One view (even in your original case) could be that it is still the user of the Socket[Stream] has the responsibility to manage the resource. There is also a strange dichotomy in managing Sockets as an external resource. First, there is classic, manual resource management. Second, there is finalization (#finalize). In Pharo you can be a bit lazy and trust on finalization. But of course it is more correct and efficient to do explicit management. I have also often wondered how and when the finalization kicks in. I have been monitoring the number of nonNil slots in the semaphore table and been triggering full GC's on long running images. My hunch is that it does work, but in many cases the GC comes too late, i.e. memory consumption is way slower that exhaustion of the table. My 2c, Sven On 06 Oct 2013, at 12:51, Norbert Hartl <norbert@hartl.name> wrote:
I took some time to analyze my current problem with external semaphores. I was just reluctant to raise the limit in my image because I want the problem solved. I logged the management of external semaphores and discovered that the table fills if a connection times out.
The problem turns out to be in
Socket>>#connectTo: hostAddress port: port waitForConnectionFor: timeout "Initiate a connection to the given port at the given host address. Waits until the connection is established or time outs." self connectNonBlockingTo: hostAddress port: port. self waitForConnectionFor: timeout ifTimedOut: [ConnectionTimedOut signal: 'Cannot connect to ' , (NetNameResolver stringFromAddress: hostAddress) , ':' , port asString]
When a socket is created three external semaphores are registered in the ExternalSemaphoreTable. If a connection times out the exception is thrown but the Socket still has his resources attached.
So e.g. in
SocketStream class>>#openConnectionToHost: hostIP port: portNumber timeout: timeout | socket | socket := Socket new. socket connectTo: hostIP port: portNumber waitForConnectionFor: timeout. ^self on: socket
it holds locally a socket (with semaphores registered) but on exception time the reference to the socket gets lost and the semaphores stay registered. The only way to unregister is on finalization time but I think it should work better. So I would add a destroy before the exception is raised.
Socket>>#connectTo: hostAddress port: port waitForConnectionFor: timeout "Initiate a connection to the given port at the given host address. Waits until the connection is established or time outs." self connectNonBlockingTo: hostAddress port: port. self waitForConnectionFor: timeout ifTimedOut: [ self destroy. ConnectionTimedOut signal: 'Cannot connect to ' , (NetNameResolver stringFromAddress: hostAddress) , ':' , port asString]
I opened a ticket at https://pharo.fogbugz.com/f/cases/11797 but I'm not sure how I am supposed to provide fixes made against a pharo2.0 image. Probably I should fix this againt 3.0 but then I'm still a 2.0 user :)
Norbert
participants (4)
-
Henrik Johansen -
Marcus Denker -
Norbert Hartl -
Sven Van Caekenberghe