Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Two different Concurrent Modification Errors resulting in null, causing game to fail to proceed to next step #6296

Open
7 tasks done
OrbMonky opened this issue Dec 20, 2024 · 4 comments
Labels
New Feature Used with the RFE tag to indicate a new feature Severity: Critical Issues described as critical as per the new issue form

Comments

@OrbMonky
Copy link

Prerequisites and Pre-Issue Checklist

  • I'm reporting the issue to the correct repository:

  • MegaMek

  • MegaMekLab

  • MekHQ

  • I've tested the issue against at least the latest MILESTONE version

  • I've asked on the MegaMek Discord about the error

  • I've reviewed the BattleTech rules and MegaMek documentation, and I've confirmed that something isn't working as intended.

  • I've searched the Github tracker and haven't found the issue listed

Severity *

Critical (Game-breaking/Crash): The game crashes or a core feature (like saving, loading, or network connection) is completely unusable.

Brief Description *

The firing phase after fleeing one of my units, suddenly a lot of errors appeared in the log.
I was presented with this;
image
and the game failed to proceed to end the firing phase, saying it was my turn to fire but with no unit selected. I think it may have been trying to have me fire from the fled unit but can't be sure.
logs.zip
The MM save where this happened is named 'gah'

Steps to Reproduce

Unable to determine, suspect fled unit may have an impact due to observed behavior remaining in firing phase and telling me to fire without a unit selected

Operating System *

Windows 10

Java Version *

17

MegaMek Suite Version *

None

Custom MegaMek Version

12/16 Logistics Artifact / 0.50.02 SNAPSHOT

Attach Files

No response

Final Checklist

  • I've checked to make sure that this issue has not already been filed
  • I'm reporting only one issue in this ticket for clarity and focus
@psikomonkie
Copy link
Collaborator

psikomonkie commented Dec 20, 2024

This may have been fixed by #6292. The issue mentions this was tested with the 12/16 artifact.

@OrbMonky (or other QA testers) Can you let me know leave a comment if this happens with the latest nightly?

Thanks,
Psi

@Sleet01
Copy link
Collaborator

Sleet01 commented Dec 20, 2024

This should be fixed in yesterday's nightly build, yeah.

This is actually happening while Princess is comparing prospective movement paths, including the attack options each would generate; it's nothing the user is doing wrong.

[Tech talk]
The root issue is that we added some new code for weapon "modes", such as Pulse (for Laser Pulse Modules), that's getting called whenever Princess checks to see what attacks a unit can make at different hexes.
But we didn't make the checks explicitly thread-safe, and they iterate over non-concurrency-aware objects in a way that makes Java nervous.
It turns out that Java will assume there is a concurrency issue anytime multiple threads operate on the same Iterator or if Iterator usage could cause concurrent modification; the exception can't actually tell when concurrent modification happens, only when it becomes possible. So it throws this exception "just in case".
The stated purpose is to expose thread-unsafe usage so the devs can make them safe.
Anyhow, this is why wrapping the offending calls in synchronize blocks is sufficient to fix the issue.
We will likely see similar problems wherever Iterators on non-concurrency-safe objects are exposed, especially for Princess to use.

@HammerGS HammerGS added New Feature Used with the RFE tag to indicate a new feature Severity: Critical Issues described as critical as per the new issue form labels Jan 4, 2025
@HoneySkull
Copy link
Collaborator

Any confirmation if this is fixed in the current nightly?

@OrbMonky
Copy link
Author

#6410 is the only CME I've seen since

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Feature Used with the RFE tag to indicate a new feature Severity: Critical Issues described as critical as per the new issue form
Projects
None yet
Development

No branches or pull requests

5 participants