From 1c0c3242eb55f7cfa91625b3db2726be1110c7bd Mon Sep 17 00:00:00 2001 From: John Brooks Date: Fri, 28 Oct 2016 16:08:16 -0600 Subject: [PATCH] core: Improve start/stop of contact connections --- core/contact.go | 88 ++++++++++++++++++++++++++++++++++----------- core/contactlist.go | 24 +++++++++++-- core/identity.go | 1 + 3 files changed, 90 insertions(+), 23 deletions(-) diff --git a/core/contact.go b/core/contact.go index a6725cc..26e5771 100644 --- a/core/contact.go +++ b/core/contact.go @@ -29,9 +29,11 @@ type Contact struct { mutex sync.Mutex events *utils.Publisher + connEnabled bool connection *protocol.OpenConnection connChannel chan *protocol.OpenConnection connClosedChannel chan struct{} + connStopped chan struct{} outboundConnAuthKnown bool @@ -40,12 +42,10 @@ type Contact struct { func ContactFromConfig(core *Ricochet, id int, data ConfigContact, events *utils.Publisher) (*Contact, error) { contact := &Contact{ - core: core, - id: id, - data: data, - events: events, - connChannel: make(chan *protocol.OpenConnection), - connClosedChannel: make(chan struct{}), + core: core, + id: id, + data: data, + events: events, } if id < 0 { @@ -62,13 +62,6 @@ func ContactFromConfig(core *Ricochet, id int, data ConfigContact, events *utils } } - // XXX Ugly and fragile way to inhibit connections - if contact.status != ricochet.Contact_REJECTED { - // XXX Should have some global trigger that starts all contact connections - // at the right time - go contact.contactConnection() - } - return contact, nil } @@ -157,6 +150,48 @@ func (c *Contact) Connection() *protocol.OpenConnection { return c.connection } +func (c *Contact) StartConnection() { + c.mutex.Lock() + defer c.mutex.Unlock() + + if c.connEnabled { + return + } + + c.connEnabled = true + c.connChannel = make(chan *protocol.OpenConnection) + c.connClosedChannel = make(chan struct{}) + c.connStopped = make(chan struct{}) + go c.contactConnection() +} + +func (c *Contact) StopConnection() { + c.mutex.Lock() + if !c.connEnabled { + c.mutex.Unlock() + return + } + stopped := c.connStopped + close(c.connChannel) + c.connChannel = nil + c.connClosedChannel = nil + c.connStopped = nil + c.mutex.Unlock() + <-stopped +} + +func (c *Contact) shouldMakeOutboundConnections() bool { + c.mutex.Lock() + defer c.mutex.Unlock() + + // Don't make connections to contacts in the REJECTED state + if c.status == ricochet.Contact_REJECTED { + return false + } + + return c.connEnabled +} + // Goroutine to handle the protocol connection for a contact. // Responsible for making outbound connections and taking over authenticated // inbound connections, running protocol handlers on the active connection, and @@ -174,23 +209,28 @@ func (c *Contact) contactConnection() { // to handler for parsing, which could let it go on any goroutine we want, if it's desirable // to put it on e.g. this routine. Hmm. + connChannel := c.connChannel + connClosedChannel := c.connClosedChannel + stopped := c.connStopped + +connectionLoop: for { // If there is no active connection, spawn an outbound connector. // A successful connection is returned via connChannel; otherwise, it will keep trying. var outboundCtx context.Context outboundCancel := func() {} - if c.connection == nil { + if c.connection == nil && c.shouldMakeOutboundConnections() { outboundCtx, outboundCancel = context.WithCancel(context.Background()) - go c.connectOutbound(outboundCtx, c.connChannel) + go c.connectOutbound(outboundCtx, connChannel) } select { - case conn, ok := <-c.connChannel: + case conn, ok := <-connChannel: outboundCancel() if !ok { // Closing connChannel exits this connection routine, for contact // deletion, exit, or some other case. - break + break connectionLoop } else if conn == nil { // Signal used to restart outbound connection attempts continue @@ -210,7 +250,7 @@ func (c *Contact) contactConnection() { continue } - case <-c.connClosedChannel: + case <-connClosedChannel: outboundCancel() c.clearConnection(nil) } @@ -218,6 +258,7 @@ func (c *Contact) contactConnection() { log.Printf("Exiting contact connection loop for %s", c.Address()) c.clearConnection(nil) + close(stopped) } // Attempt an outbound connection to the contact, retrying automatically using OnionConnector. @@ -499,20 +540,27 @@ func (c *Contact) updateContactRequest(status string) bool { // XXX also will go away during protocol API rework func (c *Contact) OnConnectionAuthenticated(conn *protocol.OpenConnection, knownContact bool) { + c.mutex.Lock() + if c.connChannel == nil { + log.Printf("Inbound connection from contact, but connections are not enabled for contact %v", c) + c.mutex.Unlock() + conn.Close() + } // XXX this is ugly if conn.Client { c.outboundConnAuthKnown = knownContact } c.connChannel <- conn + c.mutex.Unlock() } // XXX rework connection close to have a proper notification instead of this "find contact" mess. func (c *Contact) OnConnectionClosed(conn *protocol.OpenConnection) { c.mutex.Lock() - if c.connection != conn { + if c.connection != conn || c.connClosedChannel == nil { c.mutex.Unlock() return } - c.mutex.Unlock() c.connClosedChannel <- struct{}{} + c.mutex.Unlock() } diff --git a/core/contactlist.go b/core/contactlist.go index 7aae165..f3305a8 100644 --- a/core/contactlist.go +++ b/core/contactlist.go @@ -144,6 +144,7 @@ func (this *ContactList) AddContactRequest(address, name, fromName, text string) } this.events.Publish(event) + contact.StartConnection() return contact, nil } @@ -155,9 +156,14 @@ func (this *ContactList) RemoveContact(contact *Contact) error { return errors.New("Not in contact list") } - // XXX Not persisting in config - // XXX This will have to do some things to the contact itself - // eventually too, such as killing connections and other resources. + contact.StopConnection() + + config := this.core.Config.OpenWrite() + delete(config.Contacts, strconv.Itoa(contact.Id())) + if err := config.Save(); err != nil { + return err + } + delete(this.contacts, contact.Id()) event := ricochet.ContactEvent{ @@ -173,3 +179,15 @@ func (this *ContactList) RemoveContact(contact *Contact) error { return nil } + +func (this *ContactList) StartConnections() { + for _, contact := range this.Contacts() { + contact.StartConnection() + } +} + +func (this *ContactList) StopConnections() { + for _, contact := range this.Contacts() { + contact.StopConnection() + } +} diff --git a/core/identity.go b/core/identity.go index 45097f6..11109ca 100644 --- a/core/identity.go +++ b/core/identity.go @@ -43,6 +43,7 @@ func CreateIdentity(core *Ricochet) (*Identity, error) { } me.contactList = contactList + contactList.StartConnections() go me.publishService(me.privateKey) return me, nil }