Does this also fix #308?

Does this also fix #308?

Yes, and also #1108 and I believe #826, but haven't tested that one yet (may need some unit test rewriting).

Does this also fix #308?

Didn't notice that. Yes, I guess it does do that.

However, going to need some more testing. Thought the test simulation scripts ran right, but not looking like it in the sample rockets anymore.

Ok, I have it somewhat working again. Thinking about ditching the whole ScriptEngineManager/Factory... its just a mess. And GraalJs requires some custom properties with the ScriptEngine Context to execute the Java code for bindings. And even though this document (https://github.com/oracle/graaljs/blob/master/docs/user/ScriptEngine.md#setting-options-via-bindings) shows supposedly you can do it after fetching the ScriptingEngine, for me it always throws a context already created exception.
So for now its hard-coded based on the Manual method listed in the document. Not sure there are plans to support other scripting engines.. if so, maybe it can be readdressed at that time.

Now onto the next issue that isn't showing up in the tests (guess to Sibo's comment, there needs to be more unit tests).

Playing around with the simulation examples, with the above in place I'm back to being able to at least execute them now. But the java code has changed since the simulations were written and so the simulations are broken.

FlightConfiguration used to have an iterator, but now it doesn't. The script is throwing an exception when trying to use the iterator to iterator through the parts and find the fin with the name 'CUSTOM'.
Think perhaps for these scripts it would be appropriate to replace with getActiveInstances.

Ok, switching to the getActiveInstances doesn't, but getAllComponents does.

Now the next issue is when the scripting is setting the cant (c.setCantAngle(value);); the java code is throwing a ConcurrentModification exception.

But at least the script is running.

Ok, tweaks/cleanup is uploaded. I updated the scripting ork example file for the getAllComponents and updated it to use const/let instead of var for variables.

The ConcurrentModification exception is still there at this time. If you want to run it, you'll have to comment out the c.setCantAngle(value); line or click through the error dialogs that popup.

I'll note that this also happens with Java custom simulation code, so it does not appear to be related necessarily to the scripting. Will need to take a look at it.

In breaking down both the java and javascript scripts, its the finset.setCantAngle(finPosition); that is causing the issue. This is basically translating into a lot of paint events through the listeners which is where the RocketFigure.paintComponent gets fired off, and the updateShapes executed.

Removing the fireComponentChangeEvent(ComponentChangeEvent.BOTH_CHANGE); from the setCantAngle via a comment, goes from a very slow jerky simulation to very fast. Not sure what the ramifications of that are, just painting?

I did try a couple of simply techniques to try and avoid the exception

• synch and mutex around where I could find it created the set, and where it was using it in updateShape but those didn't help
• doing a copy of the set in updateShape (with sync/mutex too) in order to allow updates to happen when shape updating was happening.

Thoughts on this
a) Can the UI paint be paused during a simulation run? Or at very least update it once per step?
b) Wrap the updateShape in a ConcurrentModificationException catch for now to just ignore the exception and not promote it to the user; not ideal and doesn't help performance issues.

I tried it in 15.03 and there isn't this issue.

Removing the RocketFigure.paintComponent (or at least commenting out) does not have the ConcurrentModification issue. However, you can still see jerkiness in the simulation section. It's also still a lot slower in performance than with 15.03.

Also seeing an occasional nullpointer exception:
java.lang.NullPointerException at java.base/java.util.ArrayDeque.addLast(ArrayDeque.java:304) at java.base/java.util.ArrayList.forEach(ArrayList.java:1541) at java.base/java.util.ArrayDeque.copyElements(ArrayDeque.java:330) at java.base/java.util.ArrayDeque.<init>(ArrayDeque.java:211) at net.sf.openrocket.rocketcomponent.FlightConfiguration.getActiveComponents(FlightConfiguration.java:282) at net.sf.openrocket.rocketcomponent.FlightConfiguration.getOneLineMotorDescription(FlightConfiguration.java:465) at net.sf.openrocket.rocketcomponent.FlightConfiguration.getName(FlightConfiguration.java:446) at net.sf.openrocket.formatting.RocketDescriptorImpl.format(RocketDescriptorImpl.java:17) at

Interesting. I tried in Beta01; it too has the performance issues and throws the occasional ConcurrentModification and NullPointer exceptions in the java scripting. However, does not seem to throw them as often.

Tied to go back a few years with the unstable branch and do a compile and see if it also occurred, but not feeling like chasing down the missing jars, etc... would need to setup a java8 dev environment to really check. There have been some pretty severe changes in the underlying code between the master and unstable branch over the log years unstable has existed; it may be that it has moved things forward, but in this case it seems steps backwards.

Removing the fireComponentChangeEvent(ComponentChangeEvent.BOTH_CHANGE); from the setCantAngle via a comment, goes from a very slow jerky simulation to very fast. Not sure what the ramifications of that are, just painting?

It's not just for painting, it notifies a change in the aerodynamic properties and mass of the rocket. The only optimization you can do with is change it to fireComponentChangeEvent(ComponentChangeEvent.AERODYNAMIC_CHANGE);, because the mass of the fins doesn't change when altering the cant angle.

Can the UI paint be paused during a simulation run? Or at very least update it once per step?

I think that would be reasonable. But we should make a new issue for that, this PR is already turning out big.

It hasn't been said enough, but you've done some excellent work on this @thzero, thanks a lot!

@thzero can you updated your unstable branch with the latest openrocket unstable branch and then rebase this branch with your unstable branch? I want to check if one of the latests PRs can solve the ConcurrentModificationException.

Ok, let me see if I can get to that tonight.

Which PR are you thinking of?

Well I pulled in the recent (Ithink) and I see that the fireComponentChangeEvent(ComponentChangeEvent.AERODYNAMIC_CHANGE); was changed, and the for (RocketComponent c : status.getConfiguration().getActiveComponents()) { was changed.

I also updated the Javascripts with the getActiveComponents (just edited in the sim for now).

Tried with both the java and javascript script examples I'm still showing the ConcurrentModification exception. In all the tests I ran I pretty much got it almost immediately during the simulation.

Also, running the second simulation yields this error:

For this, you should just change line 49 in the JavaScript from const c; to var c;.

While I'm looking at the ConcurrentModificationException (this is a nasty bug btw, it's been haunting me for the past few days...) @thzero can you look at the following bug: editing either of the two simulation scripts causes two instances of JavaScript to be shown in the dropdown menu:

Also, running the second simulation yields this error:

What system? And was there anything special you did? At least an initial couple of attempts I have not been able to replicate it.

Also, running the second simulation yields this error:

For this, you should just change line 49 in the JavaScript from const c; to var c;.

Oops, yeah, I see this one. Yes, line 49 should be let c instead of 'const c'. let and const are preferred in Javascript over var since their introductions.

What system? And was there anything special you did? At least an initial couple of attempts I have not been able to replicate it.

macOS 12.4. Did nothing special, just opened up the example file, edited one of the simulations and there it was.

What system? And was there anything special you did? At least an initial couple of attempts I have not been able to replicate it.

macOS 12.4. Did nothing special, just opened up the example file, edited one of the simulations and there it was.

Mmms. Well I did the following

ant clean
ant jar

Did this both from Windows and Linux (Ubuntu) running java version OpenJDK 11.0.15.

Also ran it in debug mode on IntelliJ (Windows for now) and couldn't duplicate it.

I tried editing the scripts, removing them, adding new one in, copying and replacing the scripts, copying the script, adding multiple scripts, etc.

Mmms. Well I did the following

ant clean ant jar

Did this both from Windows and Linux (Ubuntu) running java version OpenJDK 11.0.15.

Also ran it in debug mode on IntelliJ (Windows for now) and couldn't duplicate it.

I tried editing the scripts, removing them, adding new one in, copying and replacing the scripts, copying the script, adding multiple scripts, etc.

Well since you can't replicate it, I'll see if I can fix the bug myself.

Mmms. Well I did the following
ant clean ant jar
Did this both from Windows and Linux (Ubuntu) running java version OpenJDK 11.0.15.
Also ran it in debug mode on IntelliJ (Windows for now) and couldn't duplicate it.
I tried editing the scripts, removing them, adding new one in, copying and replacing the scripts, copying the script, adding multiple scripts, etc.

Well since you can't replicate it, I'll see if I can fix the bug myself.

Ok. I don't have access to a Mac... I'm getting another Linux machine together, so was going to try there too.

Were you running from IDE? From a build jar?

Were you running from IDE? From a build jar?

Just tried it from a JAR and no problems there, so nevermind it then. I was running from my IDE when I reported the bug.

Ok, well we'll have to keep an eye on it.

Any progress on the " ConcurrentModificationException (this is a nasty bug btw, it's been haunting me for the past few days...)"? Or pointers where to look, etc... you know the codebase better than I do.

Any progress on the " ConcurrentModificationException (this is a nasty bug btw, it's been haunting me for the past few days...)"? Or pointers where to look, etc... you know the codebase better than I do.

Not that much progress. I have a basic understanding of the issue, but haven't cleared it out yet, don't know if I'll have the time this week to do so.

Okay, I finally got around solving the ConcurrentModificationException... kinda... I have to admit my solution doesn't exactly solve the root cause, but it solves the issue (but with performance drawbacks! - read further).

I have now modified InstanceMap.java and activeMotors in FlightConfiguration.java with concurrent-safe collection variants. This does affect the performance of those objects. Add-operations are about 100% more time-consuming. Remove-operations about 10%. Get-operations about 5 %. Soure.

So the question is: do we accept those performance issues to solve a bug for a feature that will be used by few people? Or a better question: can someone else fix the root issue of the ConcurrentModificationException 🙃 (potentially looking at @JoePfeiffer ?)

I promise I haven't gone away, I've just been swamped! OR does a lot of its own locking, but in a pretty haphazard manner. I can give it a look, but I'm unlikely to find a better answer than simply taking the performance hit.

I do think if there's no other answer taking the performance hit is worth it; even though scripting is little used, it's an advanced feature that I think really sets OR apart.

Probably better than throwing exceptions, if it doesn't impact anything else.

Seems like scripting got missed years ago when some of the core changes were made in the unstable branch.

Certainly less visible of an issue than the 3D issues.

Certainly less visible of an issue than the 3D issues.

🥲

Certainly less visible of an issue than the 3D issues.

Ix-nay on the ED-thray.

Let's just take the performance hit; this PR fixes a lot of bugs and brings back an interesting functionality.

Sounds good.