-5

So suppose I get two bank account A and B, and I need to atomically transfer money. The set up is the following: `

struct account{
    int64 amount;
    pthread_mutex_lock m;
}

`

here is my approach: `

bool Transfer(int from_account, int to_account, int64 amount) 
{
    pthread_lock(&account[from_account].m);
    bool ret = false;
    if(accounts[from_account].balance>=amount)
    {
        accounts[from_account].balance-=amount;
        ret = true;
    }
    pthread_unlock(&account[from_account].m);
    pthread_lock(&account[to_account].m);
    accounts[to_account].balance+=amount;
    pthread_unlock(&account[to_account].m);
    return ret;
}

`

the function transfer money from from_account to to_account, return a bool, only transfer when remaining money in the account>=ammount. Is this function an okay approach? I guess it will not cause deadlock problem but doesn't it make the whole function non-atomic? So there might be race conditions? Please help me, thanks a lot.

πάντα ῥεῖ
  • 1
  • 13
  • 116
  • 190
  • 2
    This function is not atomic. And there are race conditions. And there are deadlock scenarios. And it's not exception safe. So, no. This is not an okay approach. – Drew Dormann Jan 10 '15 at 00:35
  • 1
    Please ask a specific question. Also, what platform? – Dan Korn Jan 10 '15 at 00:36
  • _@user3799934_ Did you consider using the [c++ standards thread and synchronisation support](http://en.cppreference.com/w/cpp/thread), rather than struggling with `pthread` natively? – πάντα ῥεῖ Jan 10 '15 at 00:47
  • Thanks for comment. I'm just curious how to approach to this question with only fundamental C lock. What will be a good strategy? How about set a sequence of locking globally? – user3799934 Jan 10 '15 at 02:19

1 Answers1

0

Your code is logically broken. Regardless of the from_account's balance, to_account will win account unconditionally! (and I wanna be to_account owner :)

In this case, you must acquire both locks of two accounts, and it cause potentially deadlock problem.

The simplest way to avoid deadlock is enforcing the order of lock acquisition, for example, the smaller index account comes first.

bool Transfer(int from_account, int to_account, int64 amount) 
{
  // acquire locks (in pre-defined order)
  if (from_account < to_account)
  {
    pthread_lock(&accounts[from_account].m);
    pthread_lock(&accounts[to_account].m);
  } else {
    pthread_lock(&accounts[to_account].m);
    pthread_lock(&accounts[from_account].m);
  }
  // transfer amount
  bool ret = false;
  if (accounts[from_account].balance >= amount)
  {
    accounts[from_account].balance -= amount;
    accounts[to_account].balance += amount;
    ret = true;
  }
  // release both locks
  pthread_unlock(&accounts[from_account].m);
  pthread_unlock(&accounts[to_account].m);
  return ret;
}
yohjp
  • 2,985
  • 3
  • 30
  • 100