Bug - Fixed Stack trace "getting" a property.

Veracity

Developer
Staff member
KoLmafia threw an exception inside an ASH script. That's not supposed to happen. Turns out, there are a lot things to think about with it. I'll discuss the issues first and append the stack trace to the end.

Here are the relevant lines from the script:

Code:
    print( "*** chat must remain open until your fax is received ***" );
    cli_execute( "chat" );
    if ( !faxbot( fax_monster ) ) {
The exception was inside the "faxbot" function.

1) The cli "chat" command starts the ChatManager - but does not actually open the GUI. The ChatManager won't actually do that until it gets something that it wants to show in a chat channel.

(I noticed that back in Sept 2014, when I was integrating modern chat and older chat and whacking the chat GUI; chat used to be hola's baby, but since he'd retired by then, somebody had to take it over. I didn't understand why clicking the chat icon on the tool bar didn't immediately open the GUI; you'd have to wait until something arrived. I 'fixed' that by making it request the open channels, which made it open as soon as that message came in.)

The ChatManager will run successfully without the GUI. For example, ChatManager.openWIndow starts like this:

Code:
		if ( StaticEntity.isHeadless() || !ChatManager.isRunning || bufferKey == null )
		{
			return;
		}
And we will successfully call ChatManager.processMessage on chat messages from the Relay Browser without bothering to display them in the GUI, if you don't have the GUI open.

2) FaxRequestFrame.sendFax has this:

Code:
		// Make sure we can receive chat messages, either via KoLmafia chat or in the Relay Browser.
		if ( !( ChatManager.isRunning() || true ) )
		{
			FaxRequestFrame.statusMessage = "You must be in chat so we can receive messages from " + botName;
			KoLmafia.updateDisplay( FaxRequestFrame.statusMessage );
			return false;
		}
Notice that the conditional is actually diked out, since there is no way to tell if chat is currently running in the relay browser. You can start chat there, we will notice chat messages in RelayRequest, but if we don't see any for a while, did chat go idel, did you close the chat window, or what?

3) So, what happened with my script?

- It started ChatManager, which did not open the chat GUI
- FaxRequestFrame.requestFax sent a chat message via ChatSender.sendMessage
- ChatManager wanted to show that message and therefore created the GUI
- Asking for the value of the user setting "TabbedChatFrame" (to restore the size & location of the frame) took an NPE in the Java library - TreeMap.getEntry.

Never seen that before. Preferences stores settings in TreeMaps, which are not thread-safe. The huge bulk of activity on those maps is simply reading values, which needs no synchronization - until that happens simultaneously with changing the structure of the map by adding a new setting. Apparently that happened here.

What maps do we have?

Code:
	private static final HashMap<String, String> globalNames = new HashMap<String, String>();
	private static final TreeMap<String, Object> globalValues = new TreeMap<String, Object>();
	private static final HashMap<String,String> userNames = new HashMap<String,String>();
	private static final TreeMap<String, Object> userValues = new TreeMap<String, Object>();
The globalNames and userNames map from name to default value and are initialized exactly once at startup, when we read defaults.txt.

The globalValues and userValues map from name to current value and have get and set applied to them frequently.

Looking at how they are used within Preferences, we synchronize of "set", since that also writes out the xxx_prefs.txt file and it wouldn't do to be doing that simultaneously in multiple threads, since that would corrupt the file.

But other access to those maps - including get and remove - do not synchronize on the map.

Java TreeMap documentation says this:

Note that this implementation is not synchronized. If multiple threads access a map concurrently, and at least one of the threads modifies the map structurally, it must be synchronized externally. (A structural modification is any operation that adds or deletes one or more mappings; merely changing the value associated with an existing key is not a structural modification.) This is typically accomplished by synchronizing on some object that naturally encapsulates the map. If no such object exists, the map should be "wrapped" using the Collections.synchronizedSortedMap method. This is best done at creation time, to prevent accidental unsynchronized access to the map:

SortedMap m = Collections.synchronizedSortedMap(new TreeMap(...))
And Collections.synchronizedSortedMap has more to say.

I think we need to do this.

Code:
Unexpected error, debug log printed.
class java.lang.NullPointerException: null
java.lang.NullPointerException
	at java.util.TreeMap.getEntry(TreeMap.java:347)
	at java.util.TreeMap.get(TreeMap.java:278)
	at net.sourceforge.kolmafia.preferences.Preferences.getObject(Preferences.java:696)
	at net.sourceforge.kolmafia.preferences.Preferences.getObject(Preferences.java:690)
	at net.sourceforge.kolmafia.preferences.Preferences.getString(Preferences.java:597)
	at net.sourceforge.kolmafia.preferences.Preferences.getString(Preferences.java:490)
	at net.sourceforge.kolmafia.swingui.GenericFrame.restorePosition(GenericFrame.java:807)
	at net.sourceforge.kolmafia.swingui.GenericFrame.setVisible(GenericFrame.java:728)
	at net.sourceforge.kolmafia.CreateFrameRunnable.createFrame(CreateFrameRunnable.java:232)
	at net.sourceforge.kolmafia.chat.ChatManager.openWindow(ChatManager.java:811)
	at net.sourceforge.kolmafia.chat.ChatManager.displayMessage(ChatManager.java:353)
	at net.sourceforge.kolmafia.chat.ChatManager.processMessage(ChatManager.java:444)
	at net.sourceforge.kolmafia.chat.ChatManager.processMessages(ChatManager.java:347)
	at net.sourceforge.kolmafia.chat.ChatSender.sendRequest(ChatSender.java:224)
	at net.sourceforge.kolmafia.chat.ChatSender.sendMessage(ChatSender.java:173)
	at net.sourceforge.kolmafia.chat.ChatSender.sendMessage(ChatSender.java:124)
	at net.sourceforge.kolmafia.swingui.FaxRequestFrame.requestFax(FaxRequestFrame.java:273)
	at net.sourceforge.kolmafia.textui.RuntimeLibrary.faxbot(RuntimeLibrary.java:3848)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at net.sourceforge.kolmafia.textui.parsetree.LibraryFunction.execute(LibraryFunction.java:104)
	at net.sourceforge.kolmafia.textui.parsetree.FunctionCall.execute(FunctionCall.java:151)
	at net.sourceforge.kolmafia.textui.parsetree.Operator.applyTo(Operator.java:447)
	at net.sourceforge.kolmafia.textui.parsetree.Operation.execute(Operation.java:112)
	at net.sourceforge.kolmafia.textui.parsetree.Conditional.execute(Conditional.java:80)
	at net.sourceforge.kolmafia.textui.parsetree.If.execute(If.java:68)
	at net.sourceforge.kolmafia.textui.parsetree.BasicScope.execute(BasicScope.java:417)
	at net.sourceforge.kolmafia.textui.parsetree.UserDefinedFunction.execute(UserDefinedFunction.java:142)
	at net.sourceforge.kolmafia.textui.parsetree.FunctionCall.execute(FunctionCall.java:151)
	at net.sourceforge.kolmafia.textui.parsetree.BasicScope.execute(BasicScope.java:417)
	at net.sourceforge.kolmafia.textui.parsetree.UserDefinedFunction.execute(UserDefinedFunction.java:142)
	at net.sourceforge.kolmafia.textui.parsetree.FunctionCall.execute(FunctionCall.java:151)
	at net.sourceforge.kolmafia.textui.parsetree.BasicScope.execute(BasicScope.java:417)
	at net.sourceforge.kolmafia.textui.parsetree.Try.execute(Try.java:74)
	at net.sourceforge.kolmafia.textui.parsetree.BasicScope.execute(BasicScope.java:417)
	at net.sourceforge.kolmafia.textui.parsetree.UserDefinedFunction.execute(UserDefinedFunction.java:142)
	at net.sourceforge.kolmafia.textui.Interpreter.executeScope(Interpreter.java:401)
	at net.sourceforge.kolmafia.textui.Interpreter.execute(Interpreter.java:325)
	at net.sourceforge.kolmafia.textui.Interpreter.execute(Interpreter.java:318)
	at net.sourceforge.kolmafia.textui.command.CallScriptCommand.call(CallScriptCommand.java:256)
	at net.sourceforge.kolmafia.textui.command.CallScriptCommand.run(CallScriptCommand.java:75)
	at net.sourceforge.kolmafia.KoLmafiaCLI.doExecuteCommand(KoLmafiaCLI.java:596)
	at net.sourceforge.kolmafia.KoLmafiaCLI.executeCommand(KoLmafiaCLI.java:549)
	at net.sourceforge.kolmafia.KoLmafiaCLI.executeLine(KoLmafiaCLI.java:450)
	at net.sourceforge.kolmafia.KoLmafiaCLI.executeLine(KoLmafiaCLI.java:318)
	at net.sourceforge.kolmafia.swingui.button.LoadScriptButton$LoadScriptRunnable.run(LoadScriptButton.java:65)
	at net.sourceforge.kolmafia.RequestThread$SequencedRunnable.run(RequestThread.java:418)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:745)
 

Veracity

Developer
Staff member
Revision 17918 makes it so. Let's see how it works. So far, so good, with my testing...
 
Top