Mobile: locking enhancements

mobile: suggest improvements to locking

Author: kaloudis

mobile/mobile.go

@@ -50,6 +50,8 @@ type mobileClient struct {
 	localPrivCreateCallback  NativeCallback
 	remoteKeyReceiveCallback NativeCallback
 	authDataCallback         NativeCallback
+
+	mutex sync.Mutex
 }
 
 func newMobileClient() *mobileClient {
@@ -91,7 +93,7 @@ var (
 
 	m = make(map[string]*mobileClient)
 
-	// mMutex should always be used to guard the m map
+	// mMutex should always be used to guard the mutex map.
 	mMutex sync.RWMutex
 
 	registry = make(map[string]func(context.Context,
@@ -100,6 +102,20 @@ var (
 	interceptorLogsInitialize = false
 )
 
+// getClient returns the mobile client for the given namespace or an error if no
+// client exists.
+func getClient(nameSpace string) (*mobileClient, error) {
+	mMutex.Lock()
+	defer mMutex.Unlock()
+
+	mc, ok := m[nameSpace]
+	if !ok {
+		return nil, fmt.Errorf("unknown namespace: %v", nameSpace)
+	}
+
+	return mc, nil
+}
+
 // InitLNC sets up everything required for LNC to run including
 // signal interceptor, logs, and an instance of the mobile client.
 func InitLNC(nameSpace, debugLevel string) error {
@@ -173,17 +189,18 @@ func ConnectServer(nameSpace string, mailboxServer string, isDevServer bool,
 		}
 	}
 
-	mMutex.Lock()
-	defer mMutex.Unlock()
-
-	mc, ok := m[nameSpace]
-	if !ok {
-		return fmt.Errorf("unknown namespace: %s", nameSpace)
-	}
-
 	// Since the connection function is blocking, we need to spin it off
 	// in another goroutine here. See https://pkg.go.dev/syscall/js#FuncOf.
 	go func() {
+		mc, err := getClient(nameSpace)
+		if err != nil {
+			log.Errorf("Error getting client: %v", err)
+			return
+		}
+
+		mc.mutex.Lock()
+		defer mc.mutex.Unlock()
+
 		statusChecker, lndConnect, err := core.MailboxRPCConnection(
 			mailboxServer, pairingPhrase, localPriv, remotePub,
 			func(key *btcec.PublicKey) error {
@@ -236,27 +253,27 @@ func ConnectServer(nameSpace string, mailboxServer string, isDevServer bool,
 
 // IsConnected returns whether or not there is an active connection.
 func IsConnected(nameSpace string) (bool, error) {
-	mMutex.Lock()
-	defer mMutex.Unlock()
-
-	mc, ok := m[nameSpace]
-	if !ok {
-		return false, fmt.Errorf("unknown namespace: %s", nameSpace)
+	mc, err := getClient(nameSpace)
+	if err != nil {
+		return false, fmt.Errorf("error getting client: %v", err)
 	}
 
+	mc.mutex.Lock()
+	defer mc.mutex.Unlock()
+
 	return mc.lndConn != nil, nil
 }
 
 // Disconnect closes the RPC connection.
 func Disconnect(nameSpace string) error {
-	mMutex.Lock()
-	defer mMutex.Unlock()
-
-	mc, ok := m[nameSpace]
-	if !ok {
-		return fmt.Errorf("unknown namespace: %s", nameSpace)
+	mc, err := getClient(nameSpace)
+	if err != nil {
+		return fmt.Errorf("error getting client: %v", err)
 	}
 
+	mc.mutex.Lock()
+	defer mc.mutex.Unlock()
+
 	if mc.lndConn != nil {
 		if err := mc.lndConn.Close(); err != nil {
 			log.Errorf("Error closing RPC connection: %v", err)
@@ -269,14 +286,14 @@ func Disconnect(nameSpace string) error {
 
 // Status returns the status of the LNC RPC connection.
 func Status(nameSpace string) (string, error) {
-	mMutex.Lock()
-	defer mMutex.Unlock()
-
-	mc, ok := m[nameSpace]
-	if !ok {
-		return "", fmt.Errorf("unknown namespace: %s", nameSpace)
+	mc, err := getClient(nameSpace)
+	if err != nil {
+		return "", fmt.Errorf("error getting client: %v", err)
 	}
 
+	mc.mutex.Lock()
+	defer mc.mutex.Unlock()
+
 	if mc.statusChecker == nil {
 		return "", nil
 	}
@@ -289,14 +306,14 @@ func Status(nameSpace string) (string, error) {
 func RegisterLocalPrivCreateCallback(nameSpace string,
 	c NativeCallback) error {
 
-	mMutex.Lock()
-	defer mMutex.Unlock()
-
-	mc, ok := m[nameSpace]
-	if !ok {
-		return fmt.Errorf("unknown namespace: %s", nameSpace)
+	mc, err := getClient(nameSpace)
+	if err != nil {
+		return fmt.Errorf("error getting client: %v", err)
 	}
 
+	mc.mutex.Lock()
+	defer mc.mutex.Unlock()
+
 	mc.localPrivCreateCallback = c
 
 	return nil
@@ -307,14 +324,14 @@ func RegisterLocalPrivCreateCallback(nameSpace string,
 func RegisterRemoteKeyReceiveCallback(nameSpace string,
 	c NativeCallback) error {
 
-	mMutex.Lock()
-	defer mMutex.Unlock()
-
-	mc, ok := m[nameSpace]
-	if !ok {
-		return fmt.Errorf("unknown namespace: %s", nameSpace)
+	mc, err := getClient(nameSpace)
+	if err != nil {
+		return fmt.Errorf("error getting client: %v", err)
 	}
 
+	mc.mutex.Lock()
+	defer mc.mutex.Unlock()
+
 	mc.remoteKeyReceiveCallback = c
 
 	return nil
@@ -323,14 +340,14 @@ func RegisterRemoteKeyReceiveCallback(nameSpace string,
 // RegisterAuthDataCallback sets up the native callbacks upon
 // receiving auth data.
 func RegisterAuthDataCallback(nameSpace string, c NativeCallback) error {
-	mMutex.Lock()
-	defer mMutex.Unlock()
-
-	mc, ok := m[nameSpace]
-	if !ok {
-		return fmt.Errorf("unknown namespace: %s", nameSpace)
+	mc, err := getClient(nameSpace)
+	if err != nil {
+		return fmt.Errorf("error getting client: %v", err)
 	}
 
+	mc.mutex.Lock()
+	defer mc.mutex.Unlock()
+
 	mc.authDataCallback = c
 
 	return nil
@@ -340,14 +357,14 @@ func RegisterAuthDataCallback(nameSpace string, c NativeCallback) error {
 func InvokeRPC(nameSpace string, rpcName string, requestJSON string,
 	c NativeCallback) error {
 
-	mMutex.Lock()
-	defer mMutex.Unlock()
-
-	mc, ok := m[nameSpace]
-	if !ok {
-		return fmt.Errorf("unknown namespace: %s", nameSpace)
+	mc, err := getClient(nameSpace)
+	if err != nil {
+		return fmt.Errorf("error getting client: %v", err)
 	}
 
+	mc.mutex.Lock()
+	defer mc.mutex.Unlock()
+
 	if rpcName == "" {
 		return fmt.Errorf("param rpcName required")
 	}
@@ -385,14 +402,14 @@ func InvokeRPC(nameSpace string, rpcName string, requestJSON string,
 
 // GetExpiry returns the expiration time of the connection macaroon.
 func GetExpiry(nameSpace string) (string, error) {
-	mMutex.Lock()
-	defer mMutex.Unlock()
-
-	mc, ok := m[nameSpace]
-	if !ok {
-		return "", fmt.Errorf("unknown namespace: %s", nameSpace)
+	mc, err := getClient(nameSpace)
+	if err != nil {
+		return "", fmt.Errorf("error getting client: %v", err)
 	}
 
+	mc.mutex.Lock()
+	defer mc.mutex.Unlock()
+
 	if mc.mac == nil {
 		return "", fmt.Errorf("macaroon not obtained yet. GetExpiry" +
 			"should only be called once the connection is" +
@@ -409,14 +426,14 @@ func GetExpiry(nameSpace string) (string, error) {
 
 // IsReadOnly returns whether or not the connection macaroon is read-only.
 func IsReadOnly(nameSpace string) (bool, error) {
-	mMutex.Lock()
-	defer mMutex.Unlock()
-
-	mc, ok := m[nameSpace]
-	if !ok {
-		return false, fmt.Errorf("unknown namespace: %s", nameSpace)
+	mc, err := getClient(nameSpace)
+	if err != nil {
+		return false, fmt.Errorf("error getting client: %v", err)
 	}
 
+	mc.mutex.Lock()
+	defer mc.mutex.Unlock()
+
 	if mc.mac == nil {
 		log.Errorf("macaroon not obtained yet. IsReadOnly should " +
 			"only be called once the connection is complete")
@@ -435,16 +452,16 @@ func IsReadOnly(nameSpace string) (bool, error) {
 }
 
 // HasPermissions returns whether or not the connection macaroon
-// has a specificed permission.
+// has a specified permission.
 func HasPermissions(nameSpace, permission string) (bool, error) {
-	mMutex.Lock()
-	defer mMutex.Unlock()
-
-	mc, ok := m[nameSpace]
-	if !ok {
-		return false, fmt.Errorf("unknown namespace: %s", nameSpace)
+	mc, err := getClient(nameSpace)
+	if err != nil {
+		return false, fmt.Errorf("error getting client: %v", err)
 	}
 
+	mc.mutex.Lock()
+	defer mc.mutex.Unlock()
+
 	if permission == "" {
 		return false, nil
 	}
@@ -558,17 +575,17 @@ func validateArgs(mailboxServer, localPrivKey, remotePubKey string) error {
 // parseKeys parses the given keys from their string format and calls callback
 // functions where appropriate. NOTE: This function assumes that the parameter
 // combinations have been checked by validateArgs.
-func parseKeys(nameSpace, localPrivKey, remotePubKey string) (
-	keychain.SingleKeyECDH, *btcec.PublicKey, error) {
-
-	mMutex.Lock()
-	defer mMutex.Unlock()
+func parseKeys(nameSpace, localPrivKey,
+	remotePubKey string) (keychain.SingleKeyECDH, *btcec.PublicKey, error) {
 
-	mc, ok := m[nameSpace]
-	if !ok {
-		return nil, nil, fmt.Errorf("unknown namespace: %s", nameSpace)
+	mc, err := getClient(nameSpace)
+	if err != nil {
+		return nil, nil, fmt.Errorf("error getting client: %v", err)
 	}
 
+	mc.mutex.Lock()
+	defer mc.mutex.Unlock()
+
 	var (
 		localStaticKey  keychain.SingleKeyECDH
 		remoteStaticKey *btcec.PublicKey