Skip to content

unreferenced socket connections are not being closed on gc() #3567

Closed
amitmurthy opened this Issue Jun 28, 2013 · 11 comments

3 participants

@amitmurthy
The Julia Language member
  | | |_| | | | (_| |  |  Version 0.2.0-2259.r688b0bbb
 _/ |\__'_|_|_|\__'_|  |  Commit 688b0bbbbd 2013-06-28 10:15:18
|__/                   |  x86_64-linux-gnu

julia> open_any_tcp_port(8000)
(0x1f40,TcpServer(listening))

julia> connect("localhost", 8000)
TcpSocket(connected,0 bytes waiting)

julia> gc()

netstat still shows an existing connection to port 8000:

amitm@amitm-laptop:~/Work/julia/julia$ netstat -n | grep EST | grep 8000
tcp        0      0 127.0.0.1:60550         127.0.0.1:8000          ESTABLISHED
tcp        0      0 127.0.0.1:8000          127.0.0.1:60550         ESTABLISHED
@Keno
The Julia Language member
Keno commented Jun 28, 2013

This is deliberate, though I am not sure what the right behavior should be. Thanks for bringing this up in any case.

@amitmurthy
The Julia Language member

Any reason why this is so? If no one is holding a reference, the logical thing would be to close the socket and release all resources.

@Keno
The Julia Language member
Keno commented Jun 28, 2013

Yeah, the reason is that we're also supporting the callback interface, in which case it's perfectly natural not to hold onto the references. We did think about disallowing the callback interface by default though, so maybe this will change.

@JeffBezanson
The Julia Language member

Doesn't the stream object get passed to the callback, meaning there is still a reference to it somewhere?

@Keno
The Julia Language member
Keno commented Jun 28, 2013

Yes, we keep the reference in the libuv stream.

@JeffBezanson
The Julia Language member

It's not great that a reference exists without the user having a reference. It seems then the internal reference should be weak.

@Keno
The Julia Language member
Keno commented Jan 24, 2014

Alright, the way to do this is to associate the libuv object whenever we are entering wait and disassociating it if the wait queue of all the condition variables is empty.

@Keno Keno was assigned Jan 24, 2014
@Keno
The Julia Language member
Keno commented Feb 14, 2014

Except that doesn't work, because sometimes libuv does need to know what the julia object is, even if we are not in wait (e.g. data coming in). @JeffBezanson any preferred solutions. Should we just add an extra uint8_t in such cases that we can toggle to tell the gc whether or not to root the julia object?

EDIT: To the libuv object that is

@JeffBezanson
The Julia Language member

That sounds basically right; the julia object should be rooted if there are outside references to it (obviously), or if there are tasks waiting for it. Otherwise the reference can be dropped.

@Keno Keno added a commit to Keno/julia that referenced this issue Feb 14, 2014
@Keno Keno WIP: Fix #3567 2300e41
@Keno Keno added a commit that referenced this issue Feb 15, 2014
@Keno Keno Fix #3567
We malloc an extra byte at the beginning of the uv objects that have buffers (and thus might get a callback
that the buffer was filled even when not blocked on a read). It might be worth putting it at the end instead,
but that's kind painful too, since they are all of different length.
2f3c274
@Keno Keno added a commit that referenced this issue Feb 15, 2014
@Keno Keno Fix #3567
We malloc an extra byte at the beginning of the uv objects that have buffers (and thus might get a callback
that the buffer was filled even when not blocked on a read). It might be worth putting it at the end instead,
but that's kind painful too, since they are all of different length.
d38b98f
@Keno Keno added a commit that referenced this issue Feb 22, 2014
@Keno Keno Alternative fix for #3567
As discussed in #5825, this puts handles that should be preserved into a global ObjectIdDict. The only slightly sketchy parts are the idle and async worker invocations which I'll have to think about (we don't have a good interface for them yet, really), but it's not as simple as just putting a wait() based interface on top, since people are using them for synchronization with threaded C libraries.
933487c
@Keno Keno added a commit that referenced this issue Feb 24, 2014
@Keno Keno Alternative fix for #3567
As discussed in #5825, this puts handles that should be preserved into a global ObjectIdDict. The only slightly sketchy parts are the idle and async worker invocations which I'll have to think about (we don't have a good interface for them yet, really), but it's not as simple as just putting a wait() based interface on top, since people are using them for synchronization with threaded C libraries.
2926f1d
@Keno Keno added a commit that referenced this issue Feb 25, 2014
@Keno Keno Alternative fix for #3567
As discussed in #5825, this puts handles that should be preserved into a global ObjectIdDict. The only slightly sketchy parts are the idle and async worker invocations which I'll have to think about (we don't have a good interface for them yet, really), but it's not as simple as just putting a wait() based interface on top, since people are using them for synchronization with threaded C libraries.
2da80de
@Keno Keno added a commit that referenced this issue Feb 27, 2014
@Keno Keno Alternative fix for #3567
As discussed in #5825, this puts handles that should be preserved into a global ObjectIdDict. The only slightly sketchy parts are the idle and async worker invocations which I'll have to think about (we don't have a good interface for them yet, really), but it's not as simple as just putting a wait() based interface on top, since people are using them for synchronization with threaded C libraries.
8925c2f
@Keno Keno added a commit that referenced this issue Feb 27, 2014
@Keno Keno Alternative fix for #3567
As discussed in #5825, this puts handles that should be preserved into a global ObjectIdDict. The only slightly sketchy parts are the idle and async worker invocations which I'll have to think about (we don't have a good interface for them yet, really), but it's not as simple as just putting a wait() based interface on top, since people are using them for synchronization with threaded C libraries.
7a2893f
@Keno
The Julia Language member
Keno commented Feb 27, 2014

Fixed!

@Keno Keno closed this Feb 27, 2014
@amitmurthy
The Julia Language member

Nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.