[mynewt-newtmgr] branch master updated: nmxact - ensure connect takes priority over scan.

[cc...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

ccollins pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-newtmgr.git


The following commit(s) were added to refs/heads/master by this push:
     new 2c63642  nmxact - ensure connect takes priority over scan.
2c63642 is described below

commit 2c636426c0bcdefb86755079746872bd9a6396df
Author: Christopher Collins <cc...@apache.org>
AuthorDate: Mon Aug 28 14:16:45 2017 -0700

    nmxact - ensure connect takes priority over scan.
    
    Prior to this commit, there was a chance a preempted scan operation
    could re-acquire the master before the interrupting connector.
---
 nmxact/nmble/ble_xport.go |  2 +-
 nmxact/nmble/master.go    | 78 ++++++++++++++++++++++++++++++++---------------
 nmxact/nmxutil/sres.go    | 18 ++++++-----
 3 files changed, 65 insertions(+), 33 deletions(-)

diff --git a/nmxact/nmble/ble_xport.go b/nmxact/nmble/ble_xport.go
index 82e0549..c8a4b53 100644
--- a/nmxact/nmble/ble_xport.go
+++ b/nmxact/nmble/ble_xport.go
@@ -590,7 +590,7 @@ func (bx *BleXport) StopWaitingForMasterScan(token interface{}, err error) {
 }
 
 func (bx *BleXport) AcquireSlave(token interface{}) error {
-	return bx.slave.Acquire(token)
+	return <-bx.slave.Acquire(token)
 }
 
 func (bx *BleXport) ReleaseSlave() {
diff --git a/nmxact/nmble/master.go b/nmxact/nmble/master.go
index 9bc02c2..350a827 100644
--- a/nmxact/nmble/master.go
+++ b/nmxact/nmble/master.go
@@ -54,44 +54,74 @@ func (m *Master) unblockScanner(err error) bool {
 }
 
 func (m *Master) AcquireConnect(token interface{}) error {
-	// Stop the scanner in case it is active; connections take priority.
-	m.scanner.Preempt()
+	m.mtx.Lock()
+
+	// Append the connector to the wait queue.
+	ch := m.res.Acquire(token)
 
-	// XXX: No guarantee that the scanner won't try again before the connector.
-	// Single resource should return a channel that the caller can block on
-	// after unlocking a mutex.
-	return m.res.Acquire(token)
+	m.mtx.Unlock()
+
+	// Stop the scanner in case it is active; connections take priority.  We do
+	// this in a Goroutine so that this call doesn't block indefinitely.  We
+	// need to be reading from the acquisition channel when the scanner frees
+	// the resource.
+	go func() {
+		m.scanner.Preempt()
+	}()
+
+	// Unblocks when either:
+	// 1. Master resource becomes available.
+	// 2. Waiter aborts via call to StopWaitingConnect().
+	return <-ch
 }
 
 func (m *Master) AcquireScan(token interface{}) error {
-	// If the resource is unused, just acquire it.
-	if !m.res.Acquired() {
-		err := m.res.Acquire(token)
-		return err
+	// Gets an acquisition channel in a thread-safe manner.
+	// @return chan             Acquisition channel if scanner must wait for
+	//                              resource;
+	//                          Nil if scanner was able to acquire resource.
+	//         error            Error.
+	getChan := func() (<-chan error, error) {
+		m.mtx.Lock()
+		defer m.mtx.Unlock()
+
+		// If the resource is unused, just acquire it.
+		if !m.res.Acquired() {
+			<-m.res.Acquire(token)
+			return nil, nil
+		} else {
+			// Otherwise, wait until there are no waiting connectors.
+			if m.scanWait != nil {
+				return nil,
+					fmt.Errorf("Scanner already waiting for master resource")
+			}
+			m.scanWait = make(chan error)
+			return m.scanWait, nil
+		}
 	}
 
-	// Otherwise, wait until no one wants to connect.
-	m.mtx.Lock()
-	if m.scanWait != nil {
-		m.mtx.Unlock()
-		return fmt.Errorf("Scanner already waiting for master privileges")
+	ch, err := getChan()
+	if err != nil {
+		return err
+	}
+	if ch == nil {
+		// Resource acquired.
+		return nil
 	}
-	scanWait := make(chan error)
-	m.scanWait = scanWait
-	m.mtx.Unlock()
 
-	// Now we have to wait until someone releases the resource.  When this
-	// happens, let the releaser know when the scanner has finished acquiring
-	// the resource.  At that time, the call to Release() can unlock and
-	// return.
+	// Otherwise, we have to wait until someone releases the resource.  When
+	// this happens, let the releaser know when the scanner has finished
+	// acquiring the resource.  At that time, the call to Release() can unlock
+	// and return.
 	defer func() { m.scanAcq <- struct{}{} }()
 
 	// Wait for the resource to be released.
-	if err := <-scanWait; err != nil {
+	if err := <-ch; err != nil {
 		return err
 	}
 
-	return m.res.Acquire(token)
+	// Grab the resource; shouldn't block. */
+	return <-m.res.Acquire(token)
 }
 
 func (m *Master) Release() {
diff --git a/nmxact/nmxutil/sres.go b/nmxact/nmxutil/sres.go
index b927208..5be6640 100644
--- a/nmxact/nmxutil/sres.go
+++ b/nmxact/nmxutil/sres.go
@@ -38,13 +38,20 @@ func NewSingleResource() SingleResource {
 	return SingleResource{}
 }
 
-func (s *SingleResource) Acquire(token interface{}) error {
+// Appends an entry to the wait queue and returns a channel for the caller to
+// block on.  The caller must either read from the channel or call
+// StopWaiting().
+func (s *SingleResource) Acquire(token interface{}) <-chan error {
 	s.mtx.Lock()
 
 	if !s.acquired {
 		s.acquired = true
 		s.mtx.Unlock()
-		return nil
+
+		// Indicate immediate acquisition.
+		ch := make(chan error)
+		close(ch)
+		return ch
 	}
 
 	// XXX: Verify no duplicates.
@@ -57,12 +64,7 @@ func (s *SingleResource) Acquire(token interface{}) error {
 
 	s.mtx.Unlock()
 
-	err := <-w.c
-	if err != nil {
-		return err
-	}
-
-	return nil
+	return w.c
 }
 
 // @return                      true if a pending waiter acquired the resource;

-- 
To stop receiving notification emails like this one, please contact
['"commits@mynewt.apache.org" <co...@mynewt.apache.org>'].