2

Trying to create a little app that creates randomized passwords for the fun of it, and I'm currently at the stage of trying to make a way to let my user stop the application. However, the loop keeps on going and won't get to my scanner, but for some reason this code would work, if I took out the use of the scanner and synchronized code block.

Anyway, what is wrong with this code?:

public class Application {

public static void main(String[] args) {

    Scanner stopScanner = new Scanner(System.in);

    final PasswordGenerator pG = new PasswordGenerator();

    Thread t1 = new Thread(new Runnable(){
        @Override
        public void run() {
            try {
                pG.passwordGenerator();



            } catch (InterruptedException e) {
                e.printStackTrace();
            }

        }

    });

    t1.run();

    System.out.println("Press enter to stop!");

    stopScanner.nextLine();
    pG.shutDown();
    }
}

class PasswordGenerator{

private volatile static boolean running = true;
protected Scanner scan = new Scanner(System.in);

public void passwordGenerator() throws InterruptedException{
    synchronized(this){

        System.out.println("Select charecter set:");
        System.out.println(" 1: ABCDEF");
        System.out.println(" 2: GHIJKL");
        System.out.println(" 3: MNOPQR");
        System.out.println(" 4: TUVWXYZ");

        String charecters = scan.nextLine();

        System.out.println("Select a password length");

        int number = scan.nextInt();
        scan.nextLine();

        if(number <= 6){
            System.out.println("Number cannot be smaller or equal to six!");
        } else {
            switch(charecters){
            case "1":
                while(running == true){
                    System.out.println("placeholder");
                    Thread.sleep(1000);
                }
                break;
            case "2":

                break;
            case "3":

                break;
            case "4":

                break;
            default:
                System.out.println("No valid set chosen!");
                break;
            } 
        }
    }

}

public void shutDown(){
    running = false;
}
}
CarbonZonda
  • 167
  • 2
  • 10
  • 1
    because you have a statement that is an assignment that will always return true, you need to use == in your while loop – RAZ_Muh_Taz Nov 28 '16 at 23:57
  • 1
    Bug one, `while(running = true){` should be `while(running){` **or** `while(running == true){`. [Bug two](http://stackoverflow.com/questions/13102045/scanner-is-skipping-nextline-after-using-next-nextint-or-other-nextfoo). – Elliott Frisch Nov 28 '16 at 23:57
  • that was a mishap I made when I was re-evaluting my code, whoops. However even when I change it to == or take the = out it still won't stop the loop – CarbonZonda Nov 29 '16 at 00:01
  • You should add an if statement in your while loop to break the loop if a condition makes running = to false or just a condition to break the loop. – darkhouse Nov 29 '16 at 00:05
  • 1
    You have two threads sharing the same scanner. – xiaofeng.li Nov 29 '16 at 00:05
  • That's why you don't use boolean literals for comparison. Next time just do `if(condition)` or `if(!condition)`. – shmosel Nov 29 '16 at 00:05
  • Tried a few suggestions here ( post was updated to signify this ) but it still isn't working – CarbonZonda Nov 29 '16 at 00:13
  • It's getting worse. Using two scanners on the same input stream? Do not do it. – xiaofeng.li Nov 29 '16 at 00:14
  • I don't think the problem is with the boolean value not changing - the problem is that the code doesn't continue to my scanner so I can stop the program by pressing enter. edit: then what should I do Luke Lee, I cannot think of a solution ; i'm stumped – CarbonZonda Nov 29 '16 at 00:14
  • You need to decide which thread is responsible for getting the user's input. Why are you using multithreading anyway? – xiaofeng.li Nov 29 '16 at 00:15
  • I tried to make a loop that could be stopped using a scanner without multi threading and it didn't work – CarbonZonda Nov 29 '16 at 00:41
  • `t1.run()` should be `t1.start()`. – xiaofeng.li Nov 29 '16 at 00:57
  • Please confirm, do you want to show "Press Enter to Exit" along with the options? As answer to this question may change the code structure. @CarbonZonda – Subhra Jyoti Lahiri Nov 29 '16 at 07:03

1 Answers1

-2

Your running is volatile, and accessing volatile members is implicitly synchronized on the object containing it. Check this: http://www.javamex.com/tutorials/synchronization_volatile.shtml

Access to the (volatile) variable acts as though it is enclosed in a synchronized block, synchronized on itself

So, the lock of pG is acquired by the thread calling passwordGenerator. It never releases it. So, your main gets stuck trying to access running when it calls shutDown

The problem is not related to your scanner(s)

AhmadWabbi
  • 2,203
  • 1
  • 19
  • 30
  • "as though", means access to `running` is not guarded by an explicit lock. Or an "implicit" one. – xiaofeng.li Nov 29 '16 at 00:27
  • @LukeLee From the same page: "synchronization happens Whenever a volatile variable is accessed." – AhmadWabbi Nov 29 '16 at 00:32
  • 1
    Well, it's not the same as `synchronized`. And Java definitely do not acquire a lock on `this` before accessing `volatile` members. From the same page, in the table. `volatile` do not block. – xiaofeng.li Nov 29 '16 at 00:42