Problem with threads (concurrency)

[Igor Bo]

I am preparing for SCJP and reading K&B Study Guide - the book is really great! In the chapter 9 "Synchronizing Code (Objective 4.3)" they have this example:

package pkg;

class Account {

    private int balance = 50;

    public int getBalance() {
        return balance;
    }

    public void withdraw(int amount) {
        balance = balance - amount;
    }
}

package pkg;

public class AccountDanger implements Runnable {

    private Account acct = new Account();

    public static void main(String[] args) {
        AccountDanger ad = new AccountDanger();
        Thread one = new Thread(ad);
        Thread two = new Thread(ad);
        one.setName("Fred");
        two.setName("Lucy");
        one.start();
        two.start();
    }

    public void run() {
        for (int x = 0; x < 5; x++) {
            makeWithdrawal(10);
            if (acct.getBalance() < 0) {
                System.out.println("account is overdrawn!");
            }
        }
    }

    private void makeWithdrawal(int amt) {
        if (acct.getBalance() >= amt) {
            System.out.println(Thread.currentThread().getName()
                    + " is going to withdraw");
            try {
                Thread.sleep(500);
            } catch (InterruptedException ex) {
            }
            acct.withdraw(amt);
            System.out.println(Thread.currentThread().getName()
                    + " completes the withdrawal");
        } else {
            System.out.println("Not enough in account for "
                    + Thread.currentThread().getName() + " to withdraw " + acct.getBalance());
        }
    }
}

And I have the following output:

Can somebody explain the output, please?! I have no idea what is happening!

Running on Mac OS 10.6.5 with Java 1.0.6_22-b04-307

P.S. Did not understand the message appeared when using "AccountDanger r" instead of "AccountDanger ad" in the code(!):

We're sorry, but your post appears to contain abbreviations that we don't like people to use at the Ranch. Because JavaRanch is an international forum, many of our members are not native English speakers. For that reason, it's important that we all try to write clear, standard English, and avoid abbreviations and SMS shortcuts. See here for more of an explanation. Thanks for understanding.

If the abbreviation occurs within code, you can use code tags to post it successfully.

The specific error message is: "r" is a silly English abbreviation; use "are" instead.

[Ankit Garg]

Are you sure the code is correct?? There is only a balance of 50 in the account so withdrawal of 10 from it 10 times should not be possible.

As for the error message, generally people use sms style language like "how r u" which is hard to understand so we encourage people to use real words. In your case this system caught the r and thought it was a sms style word...

[Trivikram Kamat]

Ankit Garg wrote:
Are you sure the code is correct?? There is only a balance of 50 in the account so withdrawal of 10 from it 10 times should not be possible.

I agree with Ankit.
You may get withdrawal 6 or 7 times rarely if thread switching takes place at line:
balance = balance - amount;

But 10 times is highly improbable.

[Igor Bo]

Everything is CORRECT! I've copied the code from the NetBeans. I think there is something wrong in JVM on Mac, can't explain it another way

[Igor Bo]

Trivikram Kamat wrote:
You may get withdrawal 6 or 7 times rarely if thread switching takes place at line:
balance = balance - amount;

I think it can be only 6 withdrawals at most: after 4 withdrawals you have 10 in the balance. And then 2 threads make a 1 withdrawal each (2 withdrawal total). And the balance become -10.
There can NOT be even 7 withdrawals!!!

[Lester Burnham]

Igor Bo wrote:I think there is something wrong in JVM on Mac, can't explain it another way

It's extremely unlikely that such basic code would trigger a JRE bug. I just verified that it works as intended on OS X (assuming that allowing the account to go below zero is OK, but since the code does allow that I'm assuming it's fine).

Have you tried running the code outside of NetBeans, from the command line?

[Igor Bo]

Lester Burnham wrote:
Have you tried running the code outside of NetBeans, from the command line?

Just tried. Seems that from command line it executes correctly. But can it be 7 withdrawals? Get this in terminal:

Fred is going to withdraw
Lucy is going to withdraw
Fred completes the withdrawal
Fred is going to withdraw
Lucy completes the withdrawal
Lucy is going to withdraw
Fred completes the withdrawal
Fred is going to withdraw
Lucy completes the withdrawal
Lucy is going to withdraw
Fred completes the withdrawal
Fred is going to withdraw
Lucy completes the withdrawal
Not enough in account for Lucy to withdraw 0
Not enough in account for Lucy to withdraw 0
Fred completes the withdrawal
account is overdrawn!
Not enough in account for Fred to withdraw -10
account is overdrawn!

[Lester Burnham]

Igor Bo wrote:But can it be 7 withdrawals?

Note that the check for overdraft happens after the withdrawal - and it causes the thread to sleep.

Also, there is no synchronization of the access to the Account object.

[Khoder Wassouf]

Thank you for this example!

Well this is the reason for the Output you got:

When one of the Threads goes to Sleep another Thread starts keeping the before thread in a half a second Sleep as you chose "500"
So In this case when the Thread wakes up it continues on!.............So your its like your calling the Withdraw method twice for each time so its like your saying

for(int i0;i<10;i++) instead of for(int i=0;i<5;i++)

I copied the code and written it in a different way.............Here is the Output:

...................................................................
John is going to withdraw
New Balance for: JohnIs: 40

Mary is going to withdraw
New Balance for: MaryIs: 30

John is going to withdraw
New Balance for: JohnIs: 20

Mary is going to withdraw
New Balance for: MaryIs: 10

John is going to withdraw
New Balance for: JohnIs: 0

Mary Withdraw Terminated
Mary Withdraw Terminated
Mary Withdraw Terminated
John Withdraw Terminated
John Withdraw Terminated

............................................................

Here is the Code:

package pkg;

public class Account {

	int balance=50;
	
	public int getBalance()
	{
		return balance;
	}
	public void Withdraw(int amount)
	{
		balance=balance-amount;
	}
}

package pkg;

public class AccAmount implements Runnable{

	Account A=new Account();
	
	public static void main(String[] args) {
		// TODO Auto-generated method stub

		AccAmount A2=new AccAmount();
		Thread T1=new Thread(A2,"John");
		T1.start();
		new Thread(A2,"Mary").start();
		
	}
		public void run()
		{
			
			for(int i=0;i<5;i++)
			{
				MakeWith(10);
				if(A.getBalance()<0)
					System.out.println(Thread.currentThread().getName()+"OverDrawn!\n");
			}
		}
		public void MakeWith(int amm)
		{
			if(A.getBalance()>=amm)
			{
				A.Withdraw(amm);
				System.out.println(Thread.currentThread().getName()+"  is going to withdraw");
			    System.out.println("New Balance for: "+Thread.currentThread().getName()+"Is: "+A.balance+"\n");
			try
			{
				Thread.sleep(1000);
			}
			catch(InterruptedException ie)
			{
				ie.getLocalizedMessage();
			}
			}
			else
			{
				System.out.println(Thread.currentThread().getName()+"   Withdraw Terminated");
			}
			
		}
	}

	

[Igor Bo]

Lester Burnham wrote:

Igor Bo wrote:But can it be 7 withdrawals?

Note that the check for overdraft happens after the withdrawal - and it causes the thread to sleep.

Also, there is no synchronization of the access to the Account object.

There is no synchronization, so each of 2 threads can ask for the balance simultaneously. To make a withdrawal they need to get at least 10 in balance. So if they ask simultaneously for the balance, get 10 and make withdrawal (2 threads * 10 = 20) the result would be -10 in balance and after that they can NOT make withdrawals anymore.
4 withdrawals to have balance of 10 + 2 withdrawals made simultaneously by threads getting balance of 10 = 6 withdrawals. How can it be 7???
Didn't get note that overdraft happens after the withdrawal. I think it's not important here because we are calculating how many times program(threads) can make withdrawals, right? Or what is the maximum number of lines "**** completes the withdrawal " in the output? **** = Fred | Lucy.

[Igor Bo]

Igor Bo wrote:
Fred is going to withdraw
Lucy is going to withdraw
Fred completes the withdrawal
Fred is going to withdraw
Lucy completes the withdrawal
Lucy is going to withdraw
Fred completes the withdrawal
Fred is going to withdraw
Lucy completes the withdrawal
Lucy is going to withdraw
Fred completes the withdrawal
Fred is going to withdraw
Lucy completes the withdrawal
Not enough in account for Lucy to withdraw 0
Not enough in account for Lucy to withdraw 0
Fred completes the withdrawal
account is overdrawn!
Not enough in account for Fred to withdraw -10
account is overdrawn!

How I understand:

Where I am wrong?

[Igor Bo]

Finally, I think that the reason of 7 withdrawals is that the code doesn't use synchronized methods. Without synchronization usage of shared object become unpredictable. But it's not written anywhere officially, so this conclusion is NOT advised for exam.

[Henry Wong]

Igor Bo wrote:
There is no synchronization, so each of 2 threads can ask for the balance simultaneously. To make a withdrawal they need to get at least 10 in balance. So if they ask simultaneously for the balance, get 10 and make withdrawal (2 threads * 10 = 20) the result would be -10 in balance and after that they can NOT make withdrawals anymore.
4 withdrawals to have balance of 10 + 2 withdrawals made simultaneously by threads getting balance of 10 = 6 withdrawals. How can it be 7???
Didn't get note that overdraft happens after the withdrawal. I think it's not important here because we are calculating how many times program(threads) can make withdrawals, right? Or what is the maximum number of lines "**** completes the withdrawal " in the output? **** = Fred | Lucy.

As mentioned by a few ranchers already, there is a race condition in the account withdraw method....

public void withdraw(int amount) {  
       balance = balance - amount;  
}  

This one line isn't atomic. It loads the value of balance into a register, subtracts the value of amount from it, and then stores the value back into memory.

It is possible for the balance to be 50, and both Fred and Lucy withdraw 10, and the final balance to be 40.... ie. Fred loads the 50. Lucy loads the 50. Lucy subtracts 10 and puts the 40 back. Fred subtracts 10 and puts the 40 back.

Henry

[Henry Wong]

Side note -- which may be interesting. The sleep(500) could be keeping money from magically appearing on the account.

Without the sleep(), even this could happen (although improbable) ... Fred loads the 50. Lucy loads the 50. Lucy subtracts 10 and puts the 40 back. Lucy loads the 40. Lucy subtracts 10 and puts the 30 back. Lucy loads the 30. Lucy subtracts 10 and puts the 20 back. Lucy loads the 20. Lucy subtracts 10 and puts the 10 back. Fred subtracts 10 and puts the 40 back.

Henry

[Igor Bo]

Henry Wong wrote:
This one line isn't atomic. It loads the value of balance into a register, subtracts the value of amount from it, and then stores the value back into memory.

Henry, thank you for the reasonable answer!