Bug - Won't Fix Runnable in event dispatch thread

fronobulax

Developer
Staff member
Debug log attached. Generated on startup of r26365.

ClanManager.getStash seems to be involved so tagging @Ryo_Sangnoir.
 

Attachments

  • DEBUG_20220425.txt
    178.3 KB · Views: 2

Ryo_Sangnoir

Developer
Staff member
Looks like you have the preference debugFoxtrotRemoval set to true. Setting it to false should stop these logs being created. Looking at the code I don't think anything's crashing here: it's just commenting on an unexpected situation.

On login:
* the AdventureFrame is created, which
* updates the "last adventure" location, which
* fires a listener as the value changes, which
* calls recalculate adjustments, which
* checks to see how many of an item you have accessible, which
* makes a request to the clan stash.
 

fronobulax

Developer
Staff member
Looks like you have the preference debugFoxtrotRemoval set to true. Setting it to false should stop these logs being created. Looking at the code I don't think anything's crashing here: it's just commenting on an unexpected situation.

On login:
* the AdventureFrame is created, which
* updates the "last adventure" location, which
* fires a listener as the value changes, which
* calls recalculate adjustments, which
* checks to see how many of an item you have accessible, which
* makes a request to the clan stash.

Well I never set debugFoxtrotRemoval so perhaps, if that really is the issue, the default, at least should be changed.

In Ye Olden Days "Runnable in event dispatch thread" was a programmer's error that was fixed, stack trace or not, because it could cause users to experience an unresponsive GUI. Maybe I need to stop living in the past?
 

Ryo_Sangnoir

Developer
Staff member
The default is already false, from what I can tell.

If you're actually experiencing an unresponsive GUI, perhaps it's worth looking at, but if it's only a potential I'm not sure it's worth the time it would take to fix.

I would normally see the issue being that there is a constructor (AdventureFrame) that "does something" substantially more complicated than just setting up the class. But some brief research indicates that the constructor is where you're supposed to do things for a JFrame.

Post-that, I don't really see what else you could do. In order to compute adjustments correctly, Mafia needs information it can only get by making a request. If you don't make the request when it gets triggered, the computation might be wrong.
 

fronobulax

Developer
Staff member
The default is already false, from what I can tell.

If you're actually experiencing an unresponsive GUI, perhaps it's worth looking at, but if it's only a potential I'm not sure it's worth the time it would take to fix.

I would normally see the issue being that there is a constructor (AdventureFrame) that "does something" substantially more complicated than just setting up the class. But some brief research indicates that the constructor is where you're supposed to do things for a JFrame.

Post-that, I don't really see what else you could do. In order to compute adjustments correctly, Mafia needs information it can only get by making a request. If you don't make the request when it gets triggered, the computation might be wrong.

I have no recollection whatsoever of setting debugFoxtrotRemoval but if it is my recollection that is at fault...

Simplistically a request will run on the thread where it was submitted. This solution is to this is dispatch the run request to another thread rather than allow it run on the Swing thread. I guess maybe I need to look at this myself. I'm happy to continue the conversation once there is a PR and will add that to my list unless someone beats me to it.
 

fronobulax

Developer
Staff member
No log from 26367 and from looking at the code I cannot yet figure out the Swing thread as selected. So I will look a little more but this may be one of those things that we don't understand.
 

Ryo_Sangnoir

Developer
Staff member
The stash check code is optimised so that it doesn't try to check the stash again if it's already been checked this session. I assume this bug became visible because I broke the stash check, so it tried to do it here in the event loop, but under other circumstances something else makes it do the check, and so it doesn't try to do it twice.
 

fronobulax

Developer
Staff member
I dug into this and the stash change exposed a bug. getStash is called on the Swing thread when building a frame. getStash uses on demand initialization and "the bug" pushed initialization to the frame build. Restoring the previous getStash call moved the initialization back. I could address this but I'm not sure it should be. So won't fix seems right to me.
 

Veracity

Developer
Staff member
I don’t know if that’s a “bug”, per se. It is an undocumented assumption/requirement: stash must have been initialized before a UI element wants to use it.

Swing really only cares that a non-null LockableListModel is available when the UI element is constructed; “filling it up” via a request could be done via a parallel RequestThread, somehow.

(Fun typo from “typing” on my phone: LickableLustModel)
 
Top