From 229c93f72f7f69f046151b639972f150cde1e69b Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Sat, 29 Nov 2014 15:41:09 -0800 Subject: [PATCH 01/15] fix(net/multiconn) data race in test https://build.protocol-dev.com/job/go-ipfs.test.go.race.nofuse/276/console License: MIT Signed-off-by: Brian Tiger Chow --- net/conn/multiconn.go | 10 ++++++++++ net/conn/multiconn_test.go | 4 ++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/net/conn/multiconn.go b/net/conn/multiconn.go index 6c8752308..c5b3feafe 100644 --- a/net/conn/multiconn.go +++ b/net/conn/multiconn.go @@ -262,6 +262,16 @@ func (c *MultiConn) ID() string { return string(ids) } +func (c *MultiConn) Conns() []Conn { + c.RLock() + defer c.RUnlock() + var conns []Conn + for _, c := range c.conns { + conns = append(conns, c) + } + return conns +} + func (c *MultiConn) String() string { return String(c, "MultiConn") } diff --git a/net/conn/multiconn_test.go b/net/conn/multiconn_test.go index b511d9a3b..77aee9a8f 100644 --- a/net/conn/multiconn_test.go +++ b/net/conn/multiconn_test.go @@ -307,11 +307,11 @@ func TestMulticonnClose(t *testing.T) { ctx := context.Background() c1, c2 := setupMultiConns(t, ctx) - for _, c := range c1.conns { + for _, c := range c1.Conns() { c.Close() } - for _, c := range c2.conns { + for _, c := range c2.Conns() { c.Close() } From 65c6bd07bf47168afc62f90608353be5eb6b02a8 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Sun, 30 Nov 2014 23:33:43 -0800 Subject: [PATCH 02/15] feat(core/commands): expose commands to allow for the development of high-level interface + style: sort command list License: MIT Signed-off-by: Brian Tiger Chow --- core/commands/add.go | 2 +- core/commands/block.go | 2 +- core/commands/bootstrap.go | 2 +- core/commands/cat.go | 2 +- core/commands/config.go | 2 +- core/commands/id.go | 2 +- core/commands/ls.go | 2 +- core/commands/mount_unix.go | 2 +- core/commands/name.go | 2 +- core/commands/object.go | 2 +- core/commands/pin.go | 2 +- core/commands/refs.go | 2 +- core/commands/root.go | 30 +++++++++++++++--------------- core/commands/swarm.go | 2 +- 14 files changed, 28 insertions(+), 28 deletions(-) diff --git a/core/commands/add.go b/core/commands/add.go index 92ceeb76d..176824a9f 100644 --- a/core/commands/add.go +++ b/core/commands/add.go @@ -26,7 +26,7 @@ type AddOutput struct { Quiet bool } -var addCmd = &cmds.Command{ +var AddCmd = &cmds.Command{ Helptext: cmds.HelpText{ Tagline: "Add an object to ipfs.", ShortDescription: ` diff --git a/core/commands/block.go b/core/commands/block.go index 3efee084a..326dac2fa 100644 --- a/core/commands/block.go +++ b/core/commands/block.go @@ -18,7 +18,7 @@ type Block struct { Length int } -var blockCmd = &cmds.Command{ +var BlockCmd = &cmds.Command{ Helptext: cmds.HelpText{ Tagline: "Manipulate raw IPFS blocks", ShortDescription: ` diff --git a/core/commands/bootstrap.go b/core/commands/bootstrap.go index 4b68a67c6..eacca0352 100644 --- a/core/commands/bootstrap.go +++ b/core/commands/bootstrap.go @@ -19,7 +19,7 @@ type BootstrapOutput struct { var peerOptionDesc = "A peer to add to the bootstrap list (in the format '/')" -var bootstrapCmd = &cmds.Command{ +var BootstrapCmd = &cmds.Command{ Helptext: cmds.HelpText{ Tagline: "Show or edit the list of bootstrap peers", Synopsis: ` diff --git a/core/commands/cat.go b/core/commands/cat.go index dd5187a10..01b6142c7 100644 --- a/core/commands/cat.go +++ b/core/commands/cat.go @@ -8,7 +8,7 @@ import ( uio "github.com/jbenet/go-ipfs/unixfs/io" ) -var catCmd = &cmds.Command{ +var CatCmd = &cmds.Command{ Helptext: cmds.HelpText{ Tagline: "Show IPFS object data", ShortDescription: ` diff --git a/core/commands/config.go b/core/commands/config.go index 2e58622d0..8b15b7550 100644 --- a/core/commands/config.go +++ b/core/commands/config.go @@ -19,7 +19,7 @@ type ConfigField struct { Value interface{} } -var configCmd = &cmds.Command{ +var ConfigCmd = &cmds.Command{ Helptext: cmds.HelpText{ Tagline: "get and set IPFS config values", Synopsis: ` diff --git a/core/commands/id.go b/core/commands/id.go index 5ce38b543..3dba5957e 100644 --- a/core/commands/id.go +++ b/core/commands/id.go @@ -31,7 +31,7 @@ type IdOutput struct { ProtocolVersion string } -var idCmd = &cmds.Command{ +var IDCmd = &cmds.Command{ Helptext: cmds.HelpText{ Tagline: "Show IPFS Node ID info", ShortDescription: ` diff --git a/core/commands/ls.go b/core/commands/ls.go index 7b3db8eb5..87f65b278 100644 --- a/core/commands/ls.go +++ b/core/commands/ls.go @@ -21,7 +21,7 @@ type LsOutput struct { Objects []Object } -var lsCmd = &cmds.Command{ +var LsCmd = &cmds.Command{ Helptext: cmds.HelpText{ Tagline: "List links from an object.", ShortDescription: ` diff --git a/core/commands/mount_unix.go b/core/commands/mount_unix.go index 42763768f..a9b816ae0 100644 --- a/core/commands/mount_unix.go +++ b/core/commands/mount_unix.go @@ -22,7 +22,7 @@ const mountTimeout = time.Second // fuseNoDirectory used to check the returning fuse error const fuseNoDirectory = "fusermount: failed to access mountpoint" -var mountCmd = &cmds.Command{ +var MountCmd = &cmds.Command{ Helptext: cmds.HelpText{ Tagline: "Mounts IPFS to the filesystem (read-only)", Synopsis: ` diff --git a/core/commands/name.go b/core/commands/name.go index a50d02d43..95d1a876e 100644 --- a/core/commands/name.go +++ b/core/commands/name.go @@ -7,7 +7,7 @@ type IpnsEntry struct { Value string } -var nameCmd = &cmds.Command{ +var NameCmd = &cmds.Command{ Helptext: cmds.HelpText{ Tagline: "IPFS namespace (IPNS) tool", Synopsis: ` diff --git a/core/commands/object.go b/core/commands/object.go index b4f34e318..5e85ddc18 100644 --- a/core/commands/object.go +++ b/core/commands/object.go @@ -24,7 +24,7 @@ type Node struct { Data []byte } -var objectCmd = &cmds.Command{ +var ObjectCmd = &cmds.Command{ Helptext: cmds.HelpText{ Tagline: "Interact with ipfs objects", ShortDescription: ` diff --git a/core/commands/pin.go b/core/commands/pin.go index 341d7dd4a..35e8e62cc 100644 --- a/core/commands/pin.go +++ b/core/commands/pin.go @@ -9,7 +9,7 @@ import ( u "github.com/jbenet/go-ipfs/util" ) -var pinCmd = &cmds.Command{ +var PinCmd = &cmds.Command{ Helptext: cmds.HelpText{ Tagline: "Pin (and unpin) objects to local storage", }, diff --git a/core/commands/refs.go b/core/commands/refs.go index c977a3df8..e3cbfaab9 100644 --- a/core/commands/refs.go +++ b/core/commands/refs.go @@ -25,7 +25,7 @@ func KeyListTextMarshaler(res cmds.Response) ([]byte, error) { return []byte(s), nil } -var refsCmd = &cmds.Command{ +var RefsCmd = &cmds.Command{ Helptext: cmds.HelpText{ Tagline: "Lists link hashes from an object", ShortDescription: ` diff --git a/core/commands/root.go b/core/commands/root.go index 450b6b7e8..ae192e5c8 100644 --- a/core/commands/root.go +++ b/core/commands/root.go @@ -62,24 +62,24 @@ Use 'ipfs --help' to learn more about each command. var CommandsDaemonCmd = CommandsCmd(Root) var rootSubcommands = map[string]*cmds.Command{ - "cat": catCmd, - "ls": lsCmd, + "add": AddCmd, + "block": BlockCmd, + "bootstrap": BootstrapCmd, + "cat": CatCmd, "commands": CommandsDaemonCmd, - "name": nameCmd, - "add": addCmd, - "log": LogCmd, + "config": ConfigCmd, "diag": DiagCmd, - "pin": pinCmd, - "version": VersionCmd, - "config": configCmd, - "bootstrap": bootstrapCmd, - "mount": mountCmd, - "block": blockCmd, + "id": IDCmd, + "log": LogCmd, + "ls": LsCmd, + "mount": MountCmd, + "name": NameCmd, + "object": ObjectCmd, + "pin": PinCmd, + "refs": RefsCmd, + "swarm": SwarmCmd, "update": UpdateCmd, - "object": objectCmd, - "refs": refsCmd, - "id": idCmd, - "swarm": swarmCmd, + "version": VersionCmd, } func init() { diff --git a/core/commands/swarm.go b/core/commands/swarm.go index a450a3d94..743f46904 100644 --- a/core/commands/swarm.go +++ b/core/commands/swarm.go @@ -17,7 +17,7 @@ type stringList struct { Strings []string } -var swarmCmd = &cmds.Command{ +var SwarmCmd = &cmds.Command{ Helptext: cmds.HelpText{ Tagline: "swarm inspection tool", Synopsis: ` From 3bdb36614ed1962997ea1b34561402731ccdc61b Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Mon, 1 Dec 2014 05:52:22 -0800 Subject: [PATCH 03/15] fix(cmd/id) determine offline-ness with `!node.OnlineMode()` It's better to have one mechanism for determining whether we're offline and to improve the SnR of this mechanism over time. We presently have too many arbitrary heuristics for determining whether we're running in offline mode. TRTTD is to use polymorphism to eliminate these conditional checks. (instantiate the node with offline versions of routing, network, etc.) It'll clean up the core constructor, make it easier to create ephemeral nodes, and eliminate a class of errors. @whyrusleeping @jbenet License: MIT Signed-off-by: Brian Tiger Chow --- core/commands/id.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/commands/id.go b/core/commands/id.go index 3dba5957e..25bc9fdc3 100644 --- a/core/commands/id.go +++ b/core/commands/id.go @@ -60,7 +60,8 @@ if no peer is specified, prints out local peers info. } ctx, _ := context.WithTimeout(context.TODO(), time.Second*5) - if node.Routing == nil { + // TODO handle offline mode with polymorphism instead of conditionals + if !node.OnlineMode() { return nil, errors.New(offlineIdErrorMessage) } From 4882904c3e79ffcda59ad92df23a426190fd9f02 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Mon, 1 Dec 2014 07:27:58 -0800 Subject: [PATCH 04/15] fix: s/bootstrap rm/boostrap remove License: MIT Signed-off-by: Brian Tiger Chow --- core/commands/bootstrap.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/commands/bootstrap.go b/core/commands/bootstrap.go index eacca0352..9876615de 100644 --- a/core/commands/bootstrap.go +++ b/core/commands/bootstrap.go @@ -37,9 +37,9 @@ Running 'ipfs bootstrap' with no arguments will run 'ipfs bootstrap list'. Type: bootstrapListCmd.Type, Subcommands: map[string]*cmds.Command{ - "list": bootstrapListCmd, - "add": bootstrapAddCmd, - "rm": bootstrapRemoveCmd, + "list": bootstrapListCmd, + "add": bootstrapAddCmd, + "remove": bootstrapRemoveCmd, }, } From c91805d7bdc3e75890dd5ec6462c9c504b447460 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Mon, 1 Dec 2014 18:30:54 -0800 Subject: [PATCH 05/15] core/commands: Fixed build on Windows --- core/commands/mount_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/commands/mount_windows.go b/core/commands/mount_windows.go index 27b13381b..01bcc698c 100644 --- a/core/commands/mount_windows.go +++ b/core/commands/mount_windows.go @@ -7,7 +7,7 @@ import ( "github.com/jbenet/go-ipfs/core" ) -var mountCmd = &cmds.Command{ +var MountCmd = &cmds.Command{ Helptext: cmds.HelpText{ Tagline: "Not yet implemented on Windows", ShortDescription: "Not yet implemented on Windows. :(", From 4c7a694409828cf34dd68f1e32042e916aa3972d Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Mon, 1 Dec 2014 19:10:18 -0800 Subject: [PATCH 06/15] commands/http: Fixed client panic when sending a Request with nil 'Files' --- commands/http/client.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/commands/http/client.go b/commands/http/client.go index 3cbc41153..fa1c79fe8 100644 --- a/commands/http/client.go +++ b/commands/http/client.go @@ -48,14 +48,21 @@ func (c *client) Send(req cmds.Request) (cmds.Response, error) { } var fileReader *MultiFileReader + var reader io.Reader + if req.Files() != nil { fileReader = NewMultiFileReader(req.Files(), true) + reader = fileReader + } else { + // if we have no file data, use an empty Reader + // (http.NewRequest panics when a nil Reader is used) + reader = strings.NewReader("") } path := strings.Join(req.Path(), "/") url := fmt.Sprintf(ApiUrlFormat, c.serverAddress, ApiPath, path, query) - httpReq, err := http.NewRequest("POST", url, fileReader) + httpReq, err := http.NewRequest("POST", url, reader) if err != nil { return nil, err } From 5cb39235cc4b3425bd1eb8db7dc0f78e46f700a5 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Mon, 1 Dec 2014 23:42:14 -0800 Subject: [PATCH 07/15] commands/http: Fixed bug with client arg querystring --- commands/http/client.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/commands/http/client.go b/commands/http/client.go index fa1c79fe8..daaec0994 100644 --- a/commands/http/client.go +++ b/commands/http/client.go @@ -107,15 +107,21 @@ func getQuery(req cmds.Request) (string, error) { args := req.Arguments() argDefs := req.Command().Arguments - var argDef cmds.Argument - for i, arg := range args { - if i < len(argDefs) { - argDef = argDefs[i] + argDefIndex := 0 + + for _, arg := range args { + argDef := argDefs[argDefIndex] + // skip ArgFiles + for argDef.Type == cmds.ArgFile { + argDefIndex++ + argDef = argDefs[argDefIndex] } - if argDef.Type == cmds.ArgString { - query.Add("arg", arg) + query.Add("arg", arg) + + if len(argDefs) > argDefIndex+1 { + argDefIndex++ } } From c80a7941d144edef373278cce0d29693162e82c0 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Mon, 1 Dec 2014 23:45:18 -0800 Subject: [PATCH 08/15] commands/cli: Fixed file path formatting on Windows --- commands/cli/parse.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index b8447961c..694951bed 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -5,7 +5,7 @@ import ( "errors" "fmt" "os" - fp "path/filepath" + fp "path" "runtime" "strings" From f756088d26525bd2b9242ac06235137890f334f2 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 2 Dec 2014 00:19:22 -0800 Subject: [PATCH 09/15] fix(routing/dht) _always_ close chan on exit of FindProvidersAsync the important change here is that within FindProvidersAsync, the channel is closed using a `defer`. This ensures the channel is always closed, regardless of the path taken to exit. + misc cleanup cc @whyrusleeping @jbenet License: MIT Signed-off-by: Brian Tiger Chow --- routing/dht/dht.go | 9 +++------ routing/dht/routing.go | 30 +++++++++++++++++++----------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/routing/dht/dht.go b/routing/dht/dht.go index f76ca8f59..127cfacc5 100644 --- a/routing/dht/dht.go +++ b/routing/dht/dht.go @@ -194,24 +194,21 @@ func (dht *IpfsDHT) sendRequest(ctx context.Context, p peer.Peer, pmes *pb.Messa start := time.Now() - log.Event(ctx, "sentMessage", dht.self, p, pmes) - - rmes, err := dht.sender.SendRequest(ctx, mes) + rmes, err := dht.sender.SendRequest(ctx, mes) // respect? if err != nil { return nil, err } if rmes == nil { return nil, errors.New("no response to request") } + log.Event(ctx, "sentMessage", dht.self, p, pmes) - rtt := time.Since(start) - rmes.Peer().SetLatency(rtt) + rmes.Peer().SetLatency(time.Since(start)) rpmes := new(pb.Message) if err := proto.Unmarshal(rmes.Data(), rpmes); err != nil { return nil, err } - return rpmes, nil } diff --git a/routing/dht/routing.go b/routing/dht/routing.go index f0bfbe485..b154b270e 100644 --- a/routing/dht/routing.go +++ b/routing/dht/routing.go @@ -129,21 +129,27 @@ func (dht *IpfsDHT) FindProvidersAsync(ctx context.Context, key u.Key, count int log.Event(ctx, "findProviders", &key) peerOut := make(chan peer.Peer, count) go func() { + defer close(peerOut) + ps := newPeerSet() + // TODO may want to make this function async to hide latency provs := dht.providers.GetProviders(key) for _, p := range provs { count-- // NOTE: assuming that this list of peers is unique ps.Add(p) - peerOut <- p + select { + case peerOut <- p: + case <-ctx.Done(): + return + } if count <= 0 { return } } - wg := new(sync.WaitGroup) - peers := dht.routingTables[0].NearestPeers(kb.ConvertKey(key), AlphaValue) - for _, pp := range peers { + var wg sync.WaitGroup + for _, pp := range dht.routingTables[0].NearestPeers(kb.ConvertKey(key), AlphaValue) { wg.Add(1) go func(p peer.Peer) { defer wg.Done() @@ -156,16 +162,16 @@ func (dht *IpfsDHT) FindProvidersAsync(ctx context.Context, key u.Key, count int }(pp) } wg.Wait() - close(peerOut) }() return peerOut } func (dht *IpfsDHT) addPeerListAsync(ctx context.Context, k u.Key, peers []*pb.Message_Peer, ps *peerSet, count int, out chan peer.Peer) { - done := make(chan struct{}) + var wg sync.WaitGroup for _, pbp := range peers { + wg.Add(1) go func(mp *pb.Message_Peer) { - defer func() { done <- struct{}{} }() + defer wg.Done() // construct new peer p, err := dht.ensureConnectedToPeer(ctx, mp) if err != nil { @@ -179,15 +185,17 @@ func (dht *IpfsDHT) addPeerListAsync(ctx context.Context, k u.Key, peers []*pb.M dht.providers.AddProvider(k, p) if ps.AddIfSmallerThan(p, count) { - out <- p + select { + case out <- p: + case <-ctx.Done(): + return + } } else if ps.Size() >= count { return } }(pbp) } - for _ = range peers { - <-done - } + wg.Wait() } // Find specific Peer From de374226be0229a72e337b486588e792233ec80c Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 2 Dec 2014 00:40:50 -0800 Subject: [PATCH 10/15] fix(dht/routing) make GetProviders respect context This commit makes GetProviders (sync) respect the request context. It also amends all of GetProviders' callsites to pass a context in. This meant changing the signature of the dht's handlerfunc. I think I'll start referring to the request context as Vito Corleone. cc @whyrusleeping @jbenet License: MIT Signed-off-by: Brian Tiger Chow --- routing/dht/dht.go | 2 +- routing/dht/handlers.go | 25 ++++++++++++------------- routing/dht/providers.go | 10 +++++++--- routing/dht/providers_test.go | 2 +- routing/dht/routing.go | 2 +- 5 files changed, 22 insertions(+), 19 deletions(-) diff --git a/routing/dht/dht.go b/routing/dht/dht.go index 127cfacc5..0277f644b 100644 --- a/routing/dht/dht.go +++ b/routing/dht/dht.go @@ -161,7 +161,7 @@ func (dht *IpfsDHT) HandleMessage(ctx context.Context, mes msg.NetMessage) msg.N } // dispatch handler. - rpmes, err := handler(mPeer, pmes) + rpmes, err := handler(ctx, mPeer, pmes) if err != nil { log.Errorf("handle message error: %s", err) return nil diff --git a/routing/dht/handlers.go b/routing/dht/handlers.go index bd4b813ee..07f21f18a 100644 --- a/routing/dht/handlers.go +++ b/routing/dht/handlers.go @@ -5,20 +5,19 @@ import ( "fmt" "time" - "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/goprotobuf/proto" - + context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" + proto "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/goprotobuf/proto" + ds "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore" peer "github.com/jbenet/go-ipfs/peer" pb "github.com/jbenet/go-ipfs/routing/dht/pb" u "github.com/jbenet/go-ipfs/util" - - ds "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore" ) // The number of closer peers to send on requests. var CloserPeerCount = 4 // dhthandler specifies the signature of functions that handle DHT messages. -type dhtHandler func(peer.Peer, *pb.Message) (*pb.Message, error) +type dhtHandler func(context.Context, peer.Peer, *pb.Message) (*pb.Message, error) func (dht *IpfsDHT) handlerForMsgType(t pb.Message_MessageType) dhtHandler { switch t { @@ -39,7 +38,7 @@ func (dht *IpfsDHT) handlerForMsgType(t pb.Message_MessageType) dhtHandler { } } -func (dht *IpfsDHT) handleGetValue(p peer.Peer, pmes *pb.Message) (*pb.Message, error) { +func (dht *IpfsDHT) handleGetValue(ctx context.Context, p peer.Peer, pmes *pb.Message) (*pb.Message, error) { log.Debugf("%s handleGetValue for key: %s\n", dht.self, pmes.GetKey()) // setup response @@ -85,7 +84,7 @@ func (dht *IpfsDHT) handleGetValue(p peer.Peer, pmes *pb.Message) (*pb.Message, } // if we know any providers for the requested value, return those. - provs := dht.providers.GetProviders(u.Key(pmes.GetKey())) + provs := dht.providers.GetProviders(ctx, u.Key(pmes.GetKey())) if len(provs) > 0 { log.Debugf("handleGetValue returning %d provider[s]", len(provs)) resp.ProviderPeers = pb.PeersToPBPeers(provs) @@ -107,7 +106,7 @@ func (dht *IpfsDHT) handleGetValue(p peer.Peer, pmes *pb.Message) (*pb.Message, } // Store a value in this peer local storage -func (dht *IpfsDHT) handlePutValue(p peer.Peer, pmes *pb.Message) (*pb.Message, error) { +func (dht *IpfsDHT) handlePutValue(ctx context.Context, p peer.Peer, pmes *pb.Message) (*pb.Message, error) { dht.dslock.Lock() defer dht.dslock.Unlock() dskey := u.Key(pmes.GetKey()).DsKey() @@ -129,12 +128,12 @@ func (dht *IpfsDHT) handlePutValue(p peer.Peer, pmes *pb.Message) (*pb.Message, return pmes, err } -func (dht *IpfsDHT) handlePing(p peer.Peer, pmes *pb.Message) (*pb.Message, error) { +func (dht *IpfsDHT) handlePing(_ context.Context, p peer.Peer, pmes *pb.Message) (*pb.Message, error) { log.Debugf("%s Responding to ping from %s!\n", dht.self, p) return pmes, nil } -func (dht *IpfsDHT) handleFindPeer(p peer.Peer, pmes *pb.Message) (*pb.Message, error) { +func (dht *IpfsDHT) handleFindPeer(ctx context.Context, p peer.Peer, pmes *pb.Message) (*pb.Message, error) { resp := pb.NewMessage(pmes.GetType(), "", pmes.GetClusterLevel()) var closest []peer.Peer @@ -164,7 +163,7 @@ func (dht *IpfsDHT) handleFindPeer(p peer.Peer, pmes *pb.Message) (*pb.Message, return resp, nil } -func (dht *IpfsDHT) handleGetProviders(p peer.Peer, pmes *pb.Message) (*pb.Message, error) { +func (dht *IpfsDHT) handleGetProviders(ctx context.Context, p peer.Peer, pmes *pb.Message) (*pb.Message, error) { resp := pb.NewMessage(pmes.GetType(), pmes.GetKey(), pmes.GetClusterLevel()) // check if we have this value, to add ourselves as provider. @@ -177,7 +176,7 @@ func (dht *IpfsDHT) handleGetProviders(p peer.Peer, pmes *pb.Message) (*pb.Messa } // setup providers - providers := dht.providers.GetProviders(u.Key(pmes.GetKey())) + providers := dht.providers.GetProviders(ctx, u.Key(pmes.GetKey())) if has { providers = append(providers, dht.self) } @@ -201,7 +200,7 @@ type providerInfo struct { Value peer.Peer } -func (dht *IpfsDHT) handleAddProvider(p peer.Peer, pmes *pb.Message) (*pb.Message, error) { +func (dht *IpfsDHT) handleAddProvider(ctx context.Context, p peer.Peer, pmes *pb.Message) (*pb.Message, error) { key := u.Key(pmes.GetKey()) log.Debugf("%s adding %s as a provider for '%s'\n", dht.self, p, peer.ID(key)) diff --git a/routing/dht/providers.go b/routing/dht/providers.go index f7d491d6a..2adc20860 100644 --- a/routing/dht/providers.go +++ b/routing/dht/providers.go @@ -101,12 +101,16 @@ func (pm *ProviderManager) AddProvider(k u.Key, val peer.Peer) { } } -func (pm *ProviderManager) GetProviders(k u.Key) []peer.Peer { +func (pm *ProviderManager) GetProviders(ctx context.Context, k u.Key) []peer.Peer { gp := new(getProv) gp.k = k gp.resp = make(chan []peer.Peer) - pm.getprovs <- gp - return <-gp.resp + select { + case pm.getprovs <- gp: + return <-gp.resp + case <-ctx.Done(): + return nil + } } func (pm *ProviderManager) GetLocal() []u.Key { diff --git a/routing/dht/providers_test.go b/routing/dht/providers_test.go index c4ae53910..1ae85fbc4 100644 --- a/routing/dht/providers_test.go +++ b/routing/dht/providers_test.go @@ -15,7 +15,7 @@ func TestProviderManager(t *testing.T) { p := NewProviderManager(ctx, mid) a := u.Key("test") p.AddProvider(a, peer.WithIDString("testingprovider")) - resp := p.GetProviders(a) + resp := p.GetProviders(ctx, a) if len(resp) != 1 { t.Fatal("Could not retrieve provider.") } diff --git a/routing/dht/routing.go b/routing/dht/routing.go index b154b270e..5db218ff6 100644 --- a/routing/dht/routing.go +++ b/routing/dht/routing.go @@ -133,7 +133,7 @@ func (dht *IpfsDHT) FindProvidersAsync(ctx context.Context, key u.Key, count int ps := newPeerSet() // TODO may want to make this function async to hide latency - provs := dht.providers.GetProviders(key) + provs := dht.providers.GetProviders(ctx, key) for _, p := range provs { count-- // NOTE: assuming that this list of peers is unique From 0f6b1bc73ec213f3df82d6f5e3daba5ff0d9758f Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 2 Dec 2014 00:55:07 -0800 Subject: [PATCH 11/15] fix(dht/routing) buffer promise response to prevent resource leak When performing this "promise" pattern, it is important to provide a channel with space for one value. Otherwise the sender may block forever in the case of a receiver that decides to abandon the request. A subtle detail, but one that is important for avoiding leaked goroutines. cc @whyrusleeping @jbenet License: MIT Signed-off-by: Brian Tiger Chow License: MIT Signed-off-by: Brian Tiger Chow --- routing/dht/providers.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/routing/dht/providers.go b/routing/dht/providers.go index 2adc20860..7f70056d3 100644 --- a/routing/dht/providers.go +++ b/routing/dht/providers.go @@ -102,9 +102,10 @@ func (pm *ProviderManager) AddProvider(k u.Key, val peer.Peer) { } func (pm *ProviderManager) GetProviders(ctx context.Context, k u.Key) []peer.Peer { - gp := new(getProv) - gp.k = k - gp.resp = make(chan []peer.Peer) + gp := &getProv{ + k: k, + resp: make(chan []peer.Peer, 1), // buffered to prevent sender from blocking + } select { case pm.getprovs <- gp: return <-gp.resp From 1026244f13a1aa31fb7aa5d17bff57505ad087e7 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 2 Dec 2014 01:14:15 -0800 Subject: [PATCH 12/15] fix(net/mux) rate-limit producers by handling outgoing message synchronously License: MIT Signed-off-by: Brian Tiger Chow --- net/mux/mux.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/net/mux/mux.go b/net/mux/mux.go index d971e9054..4f54890e3 100644 --- a/net/mux/mux.go +++ b/net/mux/mux.go @@ -174,8 +174,7 @@ func (m *Muxer) handleOutgoingMessages(pid pb.ProtocolID, proto Protocol) { if !more { return } - m.Children().Add(1) - go m.handleOutgoingMessage(pid, msg) + m.handleOutgoingMessage(pid, msg) case <-m.Closing(): return @@ -185,7 +184,6 @@ func (m *Muxer) handleOutgoingMessages(pid pb.ProtocolID, proto Protocol) { // handleOutgoingMessage wraps out a message and sends it out the func (m *Muxer) handleOutgoingMessage(pid pb.ProtocolID, m1 msg.NetMessage) { - defer m.Children().Done() data, err := wrapData(m1.Data(), pid) if err != nil { From 251b916ce9b0b98e686826bcda0539a6b9517654 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 5 Dec 2014 20:46:15 -0800 Subject: [PATCH 13/15] style: readability @jbenet License: MIT Signed-off-by: Brian Tiger Chow --- routing/dht/routing.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/routing/dht/routing.go b/routing/dht/routing.go index 5db218ff6..134e54ccb 100644 --- a/routing/dht/routing.go +++ b/routing/dht/routing.go @@ -149,7 +149,8 @@ func (dht *IpfsDHT) FindProvidersAsync(ctx context.Context, key u.Key, count int } var wg sync.WaitGroup - for _, pp := range dht.routingTables[0].NearestPeers(kb.ConvertKey(key), AlphaValue) { + peers := dht.routingTables[0].NearestPeers(kb.ConvertKey(key), AlphaValue) + for _, pp := range peers { wg.Add(1) go func(p peer.Peer) { defer wg.Done() From 697453dfc278d951359f2f2d6208a67d30b120e1 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 5 Dec 2014 20:51:40 -0800 Subject: [PATCH 14/15] fix(cmd/bootstrap) s/remove/rm @jbenet License: MIT Signed-off-by: Brian Tiger Chow --- core/commands/bootstrap.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/commands/bootstrap.go b/core/commands/bootstrap.go index 9876615de..37ff44e9e 100644 --- a/core/commands/bootstrap.go +++ b/core/commands/bootstrap.go @@ -25,7 +25,7 @@ var BootstrapCmd = &cmds.Command{ Synopsis: ` ipfs bootstrap list - Show peers in the bootstrap list ipfs bootstrap add ... - Add peers to the bootstrap list -ipfs bootstrap remove ... - Removes peers from the bootstrap list +ipfs bootstrap rm ... - Removes peers from the bootstrap list `, ShortDescription: ` Running 'ipfs bootstrap' with no arguments will run 'ipfs bootstrap list'. @@ -37,9 +37,9 @@ Running 'ipfs bootstrap' with no arguments will run 'ipfs bootstrap list'. Type: bootstrapListCmd.Type, Subcommands: map[string]*cmds.Command{ - "list": bootstrapListCmd, - "add": bootstrapAddCmd, - "remove": bootstrapRemoveCmd, + "list": bootstrapListCmd, + "add": bootstrapAddCmd, + "rm": bootstrapRemoveCmd, }, } From f870948274cc4c1e0b4dc7c650955df857567d17 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 5 Dec 2014 20:54:31 -0800 Subject: [PATCH 15/15] fix: multiconn s/Conns()/getConns() @jbenet must be getConns to avoid clash with private var License: MIT Signed-off-by: Brian Tiger Chow --- net/conn/multiconn.go | 2 +- net/conn/multiconn_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/net/conn/multiconn.go b/net/conn/multiconn.go index c5b3feafe..01b91c6b1 100644 --- a/net/conn/multiconn.go +++ b/net/conn/multiconn.go @@ -262,7 +262,7 @@ func (c *MultiConn) ID() string { return string(ids) } -func (c *MultiConn) Conns() []Conn { +func (c *MultiConn) getConns() []Conn { c.RLock() defer c.RUnlock() var conns []Conn diff --git a/net/conn/multiconn_test.go b/net/conn/multiconn_test.go index 77aee9a8f..03ed2e369 100644 --- a/net/conn/multiconn_test.go +++ b/net/conn/multiconn_test.go @@ -307,11 +307,11 @@ func TestMulticonnClose(t *testing.T) { ctx := context.Background() c1, c2 := setupMultiConns(t, ctx) - for _, c := range c1.Conns() { + for _, c := range c1.getConns() { c.Close() } - for _, c := range c2.Conns() { + for _, c := range c2.getConns() { c.Close() }