Bug Mafia hangs after trying to execute (nonexistent?) script at the end of path

[ereinion]

I recently finished a HC AoSP-run, and upon completing it and choosing a new class it tried to execute the end-of-path script set in preferences. Apparently that had become set to a script included in an svn-package, which since seems to have been removed from said package, and the result was that mafia stopped execution of cli-commands. Trying to hit escape for aborting didn't help either. I waited about 15 minutes before tabbing back to the window and noticing that nothing had happened, and it's been about the same time since I tried aborting execution when I'm writing this (nothing has resolved itself yet).

Screenshot attached, in case my explanation was unclear

 

[wrldwzrd89]

I've looked at the code (for over 1 hour), and I think I know what's happening here.

KoLmafiaCLI.java:

private class CommandProcessorThread
		extends Thread
	{
		@Override
		public void run()
		{
			String line;

			do
			{
				line = KoLmafiaCLI.this.getNextLine( " > " );

				if ( KoLmafiaCLI.DEFAULT_SHELL == KoLmafiaCLI.this )
				{
					KoLmafia.forceContinue();
				}

				if ( line != null )
				{
					KoLmafiaCLI.this.isGUI = false;
					KoLmafiaCLI.this.executeLine( line );
					KoLmafiaCLI.this.isGUI = true;
				}

				if ( KoLmafiaCLI.DEFAULT_SHELL == KoLmafiaCLI.this )
				{
					KoLmafia.forceContinue();
				}
			}
			while ( line != null && KoLmafia.permitsContinue() );
		}
	}

public String getNextLine( String message )
	{
		String line = null;

		while ( line == null )
		{
			if ( message != null && KoLmafiaCLI.this.queuedLines.isEmpty() && KoLmafiaCLI.DEFAULT_SHELL == KoLmafiaCLI.this && KoLmafiaCLI.this.retriever.isAlive() )
			{
				RequestLogger.printLine();

				System.out.print( message );
			}

			if ( KoLmafiaCLI.this.queuedLines.isEmpty() && KoLmafiaCLI.this.retriever.isAlive() )
			{
				PauseObject pauser = new PauseObject();

				do
				{
					pauser.pause( 100 );
				}
				while ( KoLmafiaCLI.this.queuedLines.isEmpty() && KoLmafiaCLI.this.retriever.isAlive() );
			}

			if ( KoLmafiaCLI.this.queuedLines.isEmpty() )
			{
				return null;
			}

			if ( KoLmafiaCLI.DEFAULT_SHELL == KoLmafiaCLI.this )
			{
				RequestLogger.printLine();
			}

			synchronized ( KoLmafiaCLI.this.queuedLines )
			{
				line = KoLmafiaCLI.this.queuedLines.removeFirst();
			}
		}

		return line;
	}

Notice the condition on that loop in CommandProcessorThread. It'll terminate if line is ever null. Interestingly, in getNextLine(), if queuedLines is empty for some reason after the pauser stops pausing... null gets returned. What would cause THAT, you ask? Well... remember that liberateKing() is called by SorceressLairManager.java to handle clicking the king's prism. That in turn leads to a sequence of calls starting with this:

KoLmafiaCLI.DEFAULT_SHELL.executeLine( Preferences.getString( "kingLiberatedScript" ) );

... which of course eventually fails because kingLiberatedScript references a script file that doesn't exist. This throws a ScriptException, which is printed to the gCLI and sets the ERROR state. But... that's overridden by the CommandProcessorThread's call to forceContinue() if this instance is the default shell (which it will be).

Should that loop not look like this, instead?

			do
			{
				line = KoLmafiaCLI.this.getNextLine( " > " );

				if ( KoLmafiaCLI.DEFAULT_SHELL == KoLmafiaCLI.this )
				{
					KoLmafia.forceContinue();
				}

				if ( line != null )
				{
					KoLmafiaCLI.this.isGUI = false;
					KoLmafiaCLI.this.executeLine( line );
					KoLmafiaCLI.this.isGUI = true;
				}

				if ( KoLmafiaCLI.DEFAULT_SHELL == KoLmafiaCLI.this )
				{
					KoLmafia.forceContinue();
				}
			}
			while ( KoLmafia.permitsContinue() );

This way, if some multi-threaded correctness issue is causing this behavior (which I suspect to be the case), leading to the aforementioned NULL line, and the ComandResultProcessor stopping... it won't stop running just because it got NULL from getNextLine() after this change. That in turn would make this not happen. I know it's complex

 

[Darzil]

Doesn't removing that cause an infinite loop ?