1
\$\begingroup\$

I tried to implement a simple BankAccount class to practice providing a solution to the common Bank Account transaction data race problem. Reference to a description of such a problem can be found here.

The public API of the class:

func deposit(amount: Double)
func withdraw(amount: Double)
func currentBalance() -> Double
func sendMoney(to other: BankAccount, amount: Double)

I was thinking that we can prevent any data race from happening by using a serial queue for depositing and withdrawing money, and I went ahead and made the implementation as following (sendMoney method is excluded for now)

class BankAccount {
    
    private var balance = 0.0
    
    private let accountQueue = DispatchQueue(label: "bank.account.queue")
    
    func deposit(amount: Double) {
        accountQueue.sync(flags: .barrier) {
            balance += amount
        }
    }
    
    func withdraw(amount: Double) {
        accountQueue.sync(flags: .barrier) {
            guard balance >= amount else {
                return
            }
            balance -= amount
        }
    }
    
    func currentBalance() -> Double {
        accountQueue.sync {
            return balance
        }
    }
    
}

My first question is, are .barrier flags are unnecessary because the queue is serial anyways, and are the implementations are correct?

The second question is specifically about the correctness of the sendMoney method. I defined the method as following:

func sendMoney(to other: BankAccount, amount: Double) {
    accountQueue.sync(flags: .barrier) {
        guard balance >= amount else {
            return
        }
        other.deposit(amount: amount)
        withdrawWhileTransfering(amount: amount)
    }
}

private func withdrawWhileTransfering(amount: Double) {
    balance -= amount
}

I had to implement withdrawWhileTransfering method to call from sendMoney method, to avoid deadlock by calling regular withdraw method.

The class is like following including all the methods implemented:

class BankAccount {
    
    private var balance = 0.0
    
    private let accountQueue = DispatchQueue(label: "bank.account.queue")
    
    func deposit(amount: Double) {
        accountQueue.sync(flags: .barrier) {
            balance += amount
        }
    }
    
    func withdraw(amount: Double) {
        accountQueue.sync(flags: .barrier) {
            guard balance >= amount else {
                return
            }
            balance -= amount
        }
    }
    
    func currentBalance() -> Double {
        accountQueue.sync {
            return balance
        }
    }
    
    func sendMoney(to other: BankAccount, amount: Double) {
        accountQueue.sync(flags: .barrier) {
            guard balance >= amount else {
                return
            }
            other.deposit(amount: amount)
            withdrawWhileTransferring(amount: amount)
        }
    }
    
    private func withdrawWhileTransferring(amount: Double) {
        balance -= amount
    }
    
}

In sendMoney method, first we make sure that the current balance is greater than or equal to the amount we want to send to the other account, and then we go ahead and call deposit on the other account already. Then we withdraw the amount from the account where we send the money from. Since all this is inside a synchronous block, and the deposit method for the other will also execute serially, it feels like this would work fine, but I am not hundred percent sure.

Do you think this class would work data race free? Could we make some changes so we get rid of the private withdrawWhileTransferring method? Also do you see any performance problems with such an implementation?

PS: The API does not have to be like this by the way, if it will make the implementation better, we can introduce additional fields, such as accountID

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

You asked:

My first question is, are .barrier flags are unnecessary because the queue is serial anyways, …

Yes, if it is a serial queue, the barriers are not needed. But you can make it a concurrent queue, if you wanted, at which point the barriers would be essential.

… and are the implementations are correct?

Yes, from a synchronization perspective, it’s pretty close. You should make the writes async as there’s no reason for the caller to wait and you want to avoid deadlock risks.

From a business perspective, I suspect you want withdrawal method to return a Boolean or throw an error if there were insufficient funds. It’s not good to just silently fail. And you probably want tests to make sure that your deposits are non-negative.

Also, when dealing with currencies, make sure to use Decimal rather than Double. E.g., if your account had $1 and you remove 10 cents ten times, the final total will not quite equal 0 with Double, but will with Decimal (if you’re careful).

I had to implement withdrawWhileTransfering method to call from sendMoney method, to avoid deadlock by calling regular withdraw method.

Yeah, I get it. I might add the following to that method to make your assumptions explicit:

dispatchPrecondition(condition: .onQueue(accountQueue))

Do you think this class would work data race free?

The only problem that jumps out at me is a deadlock risk. If you simultaneously have account A send B money at the exact same time as B is sending A money, it will deadlock if all of these are synchronous. But if you make your writes asynchronous, then you should be OK.

\$\endgroup\$
2
  • \$\begingroup\$ I was reluctant to use concurrent queue and use async as I feared it could potentially result in data races. But now I understand that we could benefit from concurrency and also ensure no race condition can happen if we call async with .barrier flag. I completely ignored Decimal and error handling for the sake of concurrency but I will make sure to improve there as well. Thanks! \$\endgroup\$
    – emrepun
    Feb 7, 2022 at 16:30
  • 1
    \$\begingroup\$ This pattern of concurrent dispatch queue with writes asynchronous with barrier and reads synchronous without barrier is known as “reader-writer”. If you google "reader-writer GCD", you'll probably gets tons of good hits. \$\endgroup\$
    – Rob
    Feb 7, 2022 at 18:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.