Skip to content

Commit 7f3323d

Browse files
guggeroguggero
committed
mobile: suggest improvements to locking
1 parent 789f850 commit 7f3323d

File tree

1 file changed

+81
-139
lines changed

1 file changed

+81
-139
lines changed

‎mobile/mobile.go‎

Lines changed: 81 additions & 139 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ type mobileClient struct {
5050
localPrivCreateCallback NativeCallback
5151
remoteKeyReceiveCallback NativeCallback
5252
authDataCallback NativeCallback
53+
54+
mutex sync.Mutex
5355
}
5456

5557
func newMobileClient() *mobileClient {
@@ -91,16 +93,29 @@ var (
9193

9294
m = make(map[string]*mobileClient)
9395

94-
// mMutex should always be used to guard the mutex map
95-
mMutex sync.RWMutex
96-
mutexMap = make(map[string]sync.RWMutex)
96+
// mMutex should always be used to guard the mutex map.
97+
mMutex sync.RWMutex
9798

9899
registry = make(map[string]func(context.Context,
99100
*grpc.ClientConn, string, func(string, error)))
100101

101102
interceptorLogsInitialize = false
102103
)
103104

105+
// getClient returns the mobile client for the given namespace or an error if no
106+
// client exists.
107+
func getClient(nameSpace string) (*mobileClient, error) {
108+
mMutex.Lock()
109+
defer mMutex.Unlock()
110+
111+
mc, ok := m[nameSpace]
112+
if !ok {
113+
return nil, fmt.Errorf("unknown namespace: %v", nameSpace)
114+
}
115+
116+
return mc, nil
117+
}
118+
104119
// InitLNC sets up everything required for LNC to run including
105120
// signal interceptor, logs, and an instance of the mobile client.
106121
func InitLNC(nameSpace, debugLevel string) error {
@@ -177,21 +192,14 @@ func ConnectServer(nameSpace string, mailboxServer string, isDevServer bool,
177192
// Since the connection function is blocking, we need to spin it off
178193
// in another goroutine here. See https://pkg.go.dev/syscall/js#FuncOf.
179194
go func() {
180-
mMutex.Lock()
181-
mutex, ok := mutexMap[nameSpace]
182-
mMutex.Unlock()
183-
if !ok {
184-
log.Errorf("Unable to find mutex for namespace: %v", nameSpace)
195+
mc, err := getClient(nameSpace)
196+
if err != nil {
197+
log.Errorf("Error getting client: %v", err)
185198
return
186199
}
187-
mutex.Lock()
188-
defer mutex.Unlock()
189200

190-
mc, ok := m[nameSpace]
191-
if !ok {
192-
log.Errorf("Unknown namespace: %v", nameSpace)
193-
return
194-
}
201+
mc.mutex.Lock()
202+
defer mc.mutex.Unlock()
195203

196204
statusChecker, lndConnect, err := core.MailboxRPCConnection(
197205
mailboxServer, pairingPhrase, localPriv, remotePub,
@@ -245,38 +253,26 @@ func ConnectServer(nameSpace string, mailboxServer string, isDevServer bool,
245253

246254
// IsConnected returns whether or not there is an active connection.
247255
func IsConnected(nameSpace string) (bool, error) {
248-
mMutex.Lock()
249-
mutex, ok := mutexMap[nameSpace]
250-
mMutex.Unlock()
251-
if !ok {
252-
return false, fmt.Errorf("unable to find mutex for namespace: %v", nameSpace)
256+
mc, err := getClient(nameSpace)
257+
if err != nil {
258+
return false, fmt.Errorf("error getting client: %v", err)
253259
}
254-
mutex.Lock()
255-
defer mutex.Unlock()
256260

257-
mc, ok := m[nameSpace]
258-
if !ok {
259-
return false, fmt.Errorf("unknown namespace: %s", nameSpace)
260-
}
261+
mc.mutex.Lock()
262+
defer mc.mutex.Unlock()
261263

262264
return mc.lndConn != nil, nil
263265
}
264266

265267
// Disconnect closes the RPC connection.
266268
func Disconnect(nameSpace string) error {
267-
mMutex.Lock()
268-
mutex, ok := mutexMap[nameSpace]
269-
mMutex.Unlock()
270-
if !ok {
271-
return fmt.Errorf("unable to find mutex for namespace: %s", nameSpace)
269+
mc, err := getClient(nameSpace)
270+
if err != nil {
271+
return fmt.Errorf("error getting client: %v", err)
272272
}
273-
mutex.Lock()
274-
defer mutex.Unlock()
275273

276-
mc, ok := m[nameSpace]
277-
if !ok {
278-
return fmt.Errorf("unknown namespace: %s", nameSpace)
279-
}
274+
mc.mutex.Lock()
275+
defer mc.mutex.Unlock()
280276

281277
if mc.lndConn != nil {
282278
if err := mc.lndConn.Close(); err != nil {
@@ -290,19 +286,13 @@ func Disconnect(nameSpace string) error {
290286

291287
// Status returns the status of the LNC RPC connection.
292288
func Status(nameSpace string) (string, error) {
293-
mMutex.Lock()
294-
mutex, ok := mutexMap[nameSpace]
295-
mMutex.Unlock()
296-
if !ok {
297-
return "", fmt.Errorf("unable to find mutex for namespace: %v", nameSpace)
289+
mc, err := getClient(nameSpace)
290+
if err != nil {
291+
return "", fmt.Errorf("error getting client: %v", err)
298292
}
299-
mutex.Lock()
300-
defer mutex.Unlock()
301293

302-
mc, ok := m[nameSpace]
303-
if !ok {
304-
return "", fmt.Errorf("unknown namespace: %s", nameSpace)
305-
}
294+
mc.mutex.Lock()
295+
defer mc.mutex.Unlock()
306296

307297
if mc.statusChecker == nil {
308298
return "", nil
@@ -316,19 +306,13 @@ func Status(nameSpace string) (string, error) {
316306
func RegisterLocalPrivCreateCallback(nameSpace string,
317307
c NativeCallback) error {
318308

319-
mMutex.Lock()
320-
mutex, ok := mutexMap[nameSpace]
321-
mMutex.Unlock()
322-
if !ok {
323-
return fmt.Errorf("unable to find mutex for namespace: %s", nameSpace)
309+
mc, err := getClient(nameSpace)
310+
if err != nil {
311+
return fmt.Errorf("error getting client: %v", err)
324312
}
325-
mutex.Lock()
326-
defer mutex.Unlock()
327313

328-
mc, ok := m[nameSpace]
329-
if !ok {
330-
return fmt.Errorf("unknown namespace: %s", nameSpace)
331-
}
314+
mc.mutex.Lock()
315+
defer mc.mutex.Unlock()
332316

333317
mc.localPrivCreateCallback = c
334318

@@ -340,19 +324,13 @@ func RegisterLocalPrivCreateCallback(nameSpace string,
340324
func RegisterRemoteKeyReceiveCallback(nameSpace string,
341325
c NativeCallback) error {
342326

343-
mMutex.Lock()
344-
mutex, ok := mutexMap[nameSpace]
345-
mMutex.Unlock()
346-
if !ok {
347-
return fmt.Errorf("unable to find mutex for namespace: %s", nameSpace)
327+
mc, err := getClient(nameSpace)
328+
if err != nil {
329+
return fmt.Errorf("error getting client: %v", err)
348330
}
349-
mutex.Lock()
350-
defer mutex.Unlock()
351331

352-
mc, ok := m[nameSpace]
353-
if !ok {
354-
return fmt.Errorf("unknown namespace: %s", nameSpace)
355-
}
332+
mc.mutex.Lock()
333+
defer mc.mutex.Unlock()
356334

357335
mc.remoteKeyReceiveCallback = c
358336

@@ -362,19 +340,13 @@ func RegisterRemoteKeyReceiveCallback(nameSpace string,
362340
// RegisterAuthDataCallback sets up the native callbacks upon
363341
// receiving auth data.
364342
func RegisterAuthDataCallback(nameSpace string, c NativeCallback) error {
365-
mMutex.Lock()
366-
mutex, ok := mutexMap[nameSpace]
367-
mMutex.Unlock()
368-
if !ok {
369-
return fmt.Errorf("unable to find mutex for namespace: %s", nameSpace)
343+
mc, err := getClient(nameSpace)
344+
if err != nil {
345+
return fmt.Errorf("error getting client: %v", err)
370346
}
371-
mutex.Lock()
372-
defer mutex.Unlock()
373347

374-
mc, ok := m[nameSpace]
375-
if !ok {
376-
return fmt.Errorf("unknown namespace: %s", nameSpace)
377-
}
348+
mc.mutex.Lock()
349+
defer mc.mutex.Unlock()
378350

379351
mc.authDataCallback = c
380352

@@ -385,19 +357,13 @@ func RegisterAuthDataCallback(nameSpace string, c NativeCallback) error {
385357
func InvokeRPC(nameSpace string, rpcName string, requestJSON string,
386358
c NativeCallback) error {
387359

388-
mMutex.Lock()
389-
mutex, ok := mutexMap[nameSpace]
390-
mMutex.Unlock()
391-
if !ok {
392-
return fmt.Errorf("unable to find mutex for namespace: %s", nameSpace)
360+
mc, err := getClient(nameSpace)
361+
if err != nil {
362+
return fmt.Errorf("error getting client: %v", err)
393363
}
394-
mutex.Lock()
395-
defer mutex.Unlock()
396364

397-
mc, ok := m[nameSpace]
398-
if !ok {
399-
return fmt.Errorf("unknown namespace: %s", nameSpace)
400-
}
365+
mc.mutex.Lock()
366+
defer mc.mutex.Unlock()
401367

402368
if rpcName == "" {
403369
return fmt.Errorf("param rpcName required")
@@ -436,19 +402,13 @@ func InvokeRPC(nameSpace string, rpcName string, requestJSON string,
436402

437403
// GetExpiry returns the expiration time of the connection macaroon.
438404
func GetExpiry(nameSpace string) (string, error) {
439-
mMutex.Lock()
440-
mutex, ok := mutexMap[nameSpace]
441-
mMutex.Unlock()
442-
if !ok {
443-
return "", fmt.Errorf("unable to find mutex for namespace: %v", nameSpace)
405+
mc, err := getClient(nameSpace)
406+
if err != nil {
407+
return "", fmt.Errorf("error getting client: %v", err)
444408
}
445-
mutex.Lock()
446-
defer mutex.Unlock()
447409

448-
mc, ok := m[nameSpace]
449-
if !ok {
450-
return "", fmt.Errorf("unknown namespace: %s", nameSpace)
451-
}
410+
mc.mutex.Lock()
411+
defer mc.mutex.Unlock()
452412

453413
if mc.mac == nil {
454414
return "", fmt.Errorf("macaroon not obtained yet. GetExpiry" +
@@ -466,19 +426,13 @@ func GetExpiry(nameSpace string) (string, error) {
466426

467427
// IsReadOnly returns whether or not the connection macaroon is read-only.
468428
func IsReadOnly(nameSpace string) (bool, error) {
469-
mMutex.Lock()
470-
mutex, ok := mutexMap[nameSpace]
471-
mMutex.Unlock()
472-
if !ok {
473-
return false, fmt.Errorf("unable to find mutex for namespace: %v", nameSpace)
429+
mc, err := getClient(nameSpace)
430+
if err != nil {
431+
return false, fmt.Errorf("error getting client: %v", err)
474432
}
475-
mutex.Lock()
476-
defer mutex.Unlock()
477433

478-
mc, ok := m[nameSpace]
479-
if !ok {
480-
return false, fmt.Errorf("unknown namespace: %s", nameSpace)
481-
}
434+
mc.mutex.Lock()
435+
defer mc.mutex.Unlock()
482436

483437
if mc.mac == nil {
484438
log.Errorf("macaroon not obtained yet. IsReadOnly should " +
@@ -498,21 +452,15 @@ func IsReadOnly(nameSpace string) (bool, error) {
498452
}
499453

500454
// HasPermissions returns whether or not the connection macaroon
501-
// has a specificed permission.
455+
// has a specified permission.
502456
func HasPermissions(nameSpace, permission string) (bool, error) {
503-
mMutex.Lock()
504-
mutex, ok := mutexMap[nameSpace]
505-
mMutex.Unlock()
506-
if !ok {
507-
return false, fmt.Errorf("unable to find mutex for namespace: %v", nameSpace)
457+
mc, err := getClient(nameSpace)
458+
if err != nil {
459+
return false, fmt.Errorf("error getting client: %v", err)
508460
}
509-
mutex.Lock()
510-
defer mutex.Unlock()
511461

512-
mc, ok := m[nameSpace]
513-
if !ok {
514-
return false, fmt.Errorf("unknown namespace: %s", nameSpace)
515-
}
462+
mc.mutex.Lock()
463+
defer mc.mutex.Unlock()
516464

517465
if permission == "" {
518466
return false, nil
@@ -627,22 +575,16 @@ func validateArgs(mailboxServer, localPrivKey, remotePubKey string) error {
627575
// parseKeys parses the given keys from their string format and calls callback
628576
// functions where appropriate. NOTE: This function assumes that the parameter
629577
// combinations have been checked by validateArgs.
630-
func parseKeys(nameSpace, localPrivKey, remotePubKey string) (
631-
keychain.SingleKeyECDH, *btcec.PublicKey, error) {
578+
func parseKeys(nameSpace, localPrivKey,
579+
remotePubKey string) (keychain.SingleKeyECDH, *btcec.PublicKey, error) {
632580

633-
mMutex.Lock()
634-
mutex, ok := mutexMap[nameSpace]
635-
mMutex.Unlock()
636-
if !ok {
637-
return nil, nil, fmt.Errorf("unable to find mutex for namespace: %v", nameSpace)
581+
mc, err := getClient(nameSpace)
582+
if err != nil {
583+
return nil, nil, fmt.Errorf("error getting client: %v", err)
638584
}
639-
mutex.Lock()
640-
defer mutex.Unlock()
641585

642-
mc, ok := m[nameSpace]
643-
if !ok {
644-
return nil, nil, fmt.Errorf("unknown namespace: %s", nameSpace)
645-
}
586+
mc.mutex.Lock()
587+
defer mc.mutex.Unlock()
646588

647589
var (
648590
localStaticKey keychain.SingleKeyECDH

0 commit comments

Comments
 (0)