Giter Site home page Giter Site logo

Comments (22)

anacrolix avatar anacrolix commented on August 15, 2024

The UDP socket is not truly closed until all Conns are closed. If the process is destroyed, that is sufficient. It may be that you need to alter your torrent.Client to use a dynamic port if you are running more than one Client on a single host.

from utp.

axet avatar axet commented on August 15, 2024

I'm using one client, but I need to restart it. And when I Close/Create I'm getting this error.

from utp.

anacrolix avatar anacrolix commented on August 15, 2024

I should expose Destroy, that forces immediate closure of utp.Conn's bound to the Socket, and a Close of the underlying net.UDPConn. Feel free to make PR for something like this or I'll get around to it.

from utp.

anacrolix avatar anacrolix commented on August 15, 2024

I don't see this behaviour on basic tests, even if I force reuse of the same port. What OS? Can you provide a test case?

from utp.

axet avatar axet commented on August 15, 2024

Mac OSX, Android.

You have to have some active connections, so utp.Socket.Close() will return from lazyDestroy() on len(s.conns) != 0.

Easy to reproduce with your go.torrent client. Or create some code like this:

package main

import (
    "fmt"
    "github.com/anacrolix/utp"
)

func main() {
    s, err := utp.NewSocket("udp", ":5555")
    fmt.Println(err)

    go func() {
        s.Accept()
    }()

    c, err := utp.NewSocket("udp", ":4444")
    fmt.Println(err)

    c.Dial(":5555")

    s.Close()

    s, err = utp.NewSocket("udp", ":5555")
    fmt.Println(err)

    s.Close()
    c.Close()
}

from utp.

anacrolix avatar anacrolix commented on August 15, 2024

@axet Yes but this is currently by intention. In your test code above, you've leaked the net.Conns returned from c.Dial, and s.Accept(). The current design is that the udp net.PacketConn won't be closed until both the Socket, and all its Conns are closed. If you could make a test case where this occurs, and yet the udp PacketConn isn't closed?

from utp.

axet avatar axet commented on August 15, 2024

Current uTP socket implementation has side effect where you create object, close it and unable to create it again.

This test case leaking connections by purpose, since in real world you not able to control other side so connection can stay alive for some time.

Creating test case one one machine where you create and close all connections has no point. Because next we have to create test case where we emulate remote host which keeps connection. And yet, it is possible to create such test. We just need close socket, but not send anything. Result: all sockets will be closed, but few conn's will be alive.

from utp.

anacrolix avatar anacrolix commented on August 15, 2024

I created a test, where the remote socket abruptly disappears. After 15s (the send timeout for the FIN sent by the local Conn), the local Socket is destroyed, and the underlying PacketConn closed. Interestingly, even if I force the PacketConn to be closed right away, I can't listen successfully right away, I get bind: address already in use. Presumably the kernel has to flush any queued packets both in and out before it allows this.

from utp.

axet avatar axet commented on August 15, 2024

This happens because golang works strange with sockets, It should add use SO_REUSEADDR.

This library works:

https://github.com/kavu/go_reuseport

from utp.

AudriusButkevicius avatar AudriusButkevicius commented on August 15, 2024

This is UDP, reuseaddr has no meaning in context of udp

from utp.

anacrolix avatar anacrolix commented on August 15, 2024

Well I've added a test, that should be adaptable to the desired behaviour for Close, or to test a new method like Destroy or something.

from utp.

anacrolix avatar anacrolix commented on August 15, 2024

Have you tried waiting 15-20s for your torrent.Client's UDP connection to be lazily destroyed and reclaimed by the OS? Does that work? Alternatively, restart the process? Is that feasible?

from utp.

AudriusButkevicius avatar AudriusButkevicius commented on August 15, 2024

Not sure what the thumbs down is for, reuseport for UDP is only useful for multicast subscriptions, allowing multiple processes to subscribe. It also has no effect for nornal UDP connections.

from utp.

axet avatar axet commented on August 15, 2024

@AudriusButkevicius you were right. I have to read about UDP more. I expect this code will throw exception.

import socket

sock = socket.socket(socket.AF_INET,  socket.SOCK_DGRAM)
sock.bind(("0.0.0.0", 5005))

sock = socket.socket(socket.AF_INET,  socket.SOCK_DGRAM)
sock.bind(("0.0.0.0", 5005))

while True:
 data, addr = sock.recvfrom(1024)
 print "received message:", data

from utp.

axet avatar axet commented on August 15, 2024

@anacrolix Last time I tried it failed, and I moved to automatic port ":0".

from utp.

anacrolix avatar anacrolix commented on August 15, 2024

There are 3 solutions here:

  1. Set the default torrent.Client port to 0, so it's dynamically allocated. This has the downside that the client moves ports between restarts, so the swarms knowledge of its existence starts again from scratch.
  2. Forcibly close the UDP socket that utp wraps. This doesn't guarantee that the OS will relinquish it immediately. It's also a behaviour change to the tests, and package torrent unless it's a new method, and utp.Conns will not necessarily clean up tidily with their peers.
  3. Use SO_REUSEPORT. This means that if more than one DHT or torrent Client bind to the same UDP port, the traffic will be split, breaking the behaviour.

Solution 2 seems the best, and I think matches @axet's requirements. There'd be some work to ensure the immediate close triggers all the Socket's utp.Conn to send FINs wherever possible before the net.PacketConn is closed and that becomes no longer possible.

If there's a consensus, I'll create a new method that closes immediately with the above behaviour. I'm not sure what's a good name, Socket.Destroy, CloseNow, DestroyNow, CloseImmediately, something like that. Is there a precedent for naming this?

from utp.

prettymuchbryce avatar prettymuchbryce commented on August 15, 2024

I have been running into this same issue. #2 sounds like the right solution to me. I think this method should attempt to block until the underlying socket has been closed by the OS.

After doing some experimenting locally (Darwin) it looks like Go won't actually call the close() syscall until all reads/writes on the socket have completed.

Example: https://gist.github.com/prettymuchbryce/ee7a74fe1905dd29190e8f91dc822e88

After the FINs are sent we could make sure with a sync.Waitgroup or some other mechanism that reads/writes from utp.Socket.reader() and utp.Socket.WriteTo() have returned before unblocking CloseNow.

from utp.

anacrolix avatar anacrolix commented on August 15, 2024

A blocking CloseNow with the behaviour you described sounds ideal. A test that creates a Socket, then calls CloseNow and immediately tries to create a new Socket on the same address, similar to what is happening in your gist should also be included. If someone wants to do a PR for this, that would be great. I might not have time or the resources to do it myself for a week or two.

from utp.

anacrolix avatar anacrolix commented on August 15, 2024

I modified the package torrent client tests to reuse addresses and didn't run into the problem described by @axet. I think the tests just aren't putting enough load on the system. @axet Could you try with Socket.CloseNow, per 59dfcf2 in your torrent code and see if that fixes your problem?

from utp.

axet avatar axet commented on August 15, 2024

@anacrolix still crashing

from utp.

anacrolix avatar anacrolix commented on August 15, 2024

Would that be

if cl.utpSock != nil {
    cl.utpSock.CloseNow()
}

in Client.Close() ?

from utp.

axet avatar axet commented on August 15, 2024

@anacrolix it works.

from utp.

Related Issues (19)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.