[Bug]: canSeeToken() crashes when calling without a broadcast() before

[Lector]

Describe the Bug

I have a pretty complex macro to determine the illumination of a token. This worked very well in 1.12.0. According to the changes in the lighting system i had to make some small changes to make it work again. This changes consisted of reading the campaign lights via json.

I mentioned that this script now tends to crash when calling canSeeToken() throwing a java.util.ConcurrentModificationException. When entering [h: broadcast("")] before canSeeToken() it works (most of the time).

To Reproduce

LightDemo.zip

1. Download the LightDemo.zip, which contains a campaign file. Open the campaign.
2. Impersonate a PC token (e.g. Geron)
3. Select the Orc
4. Click on the macro "checkIllumination" in the campaign panel. It should now print the visibility of the selected token for the impersonated token into the chat.
5. Open getIllumination@Lib:lighting. Remove line 112 which consists of a [h: broadcast("")].
6. Repeat steps 2-4. The macro should now crash with a java exception.

Expected Behaviour

I would like my macro to behave like before. canSeeToken() should return what it is supposed to without causing any java exceptions.

Screenshots

No response

MapTool Info

1.13.0-beta.1

Desktop

Windows 10

Additional Context

No response

[kwvanderlinde]

The issue is a lack of thread-safety. It's a pervasive problem that we happen to get away with a lot of the time.

Some details:

• The various caches in ZoneView aren't thread-safe. In this case, we're running into an issue with illuminationCache, but it's not really unique in this regard.
• ServerMessageHandler handles all messages on the message receive thread, and doesn't delegate to the Swing thread. In this case, it's the UpdateTokenPropertyMsg giving us grief (specifically setSightType I believe), but the other messages are similar. Contrast this with ClientMessageHandler which runs most if not everything on the Swing thread.

We can avoid this issue by changing ServerMessageHandler to delegate to the Swing thread. But we should also take a look at all the threads we create and should generally try to rein those in. Not sure if there's existing discussion around that.

[Lector]

I removed my broadcast("") and did not mention any issues. I consider this as fixed :)