Removing an object from a `WrapObjectSet` in MATLAB leads to a crash during `model.finalizeConnections()`

[carmichaelong]

Based on this forum post: https://simtk.org/plugins/phpBB/viewtopicPhpbb.php?f=91&t=17865&p=0&start=0&view=&sid=3328450be9628178128448259ef44f95

Simplified model for testing is provided here: pendulum.zip

MATLAB code crashes.

import org.opensim.modeling.*;

m = Model('pendulum.osim');
b = m.getBodySet().get(0);

wrapObjSet = b.get_WrapObjectSet();
wrapObjSet.remove(0);

m.finalizeConnections();

Similar C++ code does not crash.

Model m = Model("pendulum.osim");
WrapObjectSet set = m.getBodySet().get(0).get_WrapObjectSet();
set.remove(0);

m.finalizeConnections();

[carmichaelong]

@aymanhab here's the MATLAB crash log

MATLAB Log File
------------------------------------------------ 


--------------------------------------------------------------------------------
          Segmentation violation detected at 2024-05-21 14:35:54 -0700
--------------------------------------------------------------------------------

Configuration:
  Crash Decoding           : Disabled - No sandbox or build area path
  Crash Mode               : continue (default)
  Default Encoding         : UTF-8
  Deployed                 : false
  Graphics Driver          : Unknown hardware 
  Java Version             : Java 1.8.0_202-b08 with Oracle Corporation Java HotSpot(TM) 64-Bit Server VM mixed mode
  MATLAB Architecture      : maci64
  MATLAB Entitlement ID    : 5658597
  MATLAB Root              : /Applications/MATLAB_R2021b.app
  MATLAB Version           : 9.11.0.1837725 (R2021b) Update 2
  OpenGL                   : hardware
  Operating System         : Mac OS Version 14.2.1 (Build 23C71)
  Process ID               : 57377
  Processor ID             : x86 Family 6 Model 44 Stepping 0, GenuineIntel
  Session Key              : 4c9ec8da-c36e-4f78-8411-3610cb462e3c
  Window System            : Quartz

Fault Count: 1


Abnormal termination:
Segmentation violation

Current Thread: 'MCR 0 interpreter thread' id 0x306677000

Register State (from fault):
  RAX = 5000000000000000  RBX = 0000000000000000
  RCX = 000060000c446740  RDX = 0000000000000000
  RSP = 00000003066725c0  RBP = 0000000306672640
  RSI = 0000000000000000  RDI = 00007f96015d00f0

   R8 = 0000000000000002   R9 = 000000000000013b
  R10 = 0000000000000020  R11 = 0000000000000139
  R12 = 00007f9603979801  R13 = 000060000baf7800
  R14 = 00007f96015d00f0  R15 = 00007f96015d0120

  RIP = 000000020ead4648  RFL = 0000000000000202

   CS = 000000000000002b   FS = 0000000000000000   GS = 0000000000000000

Stack Trace (from fault):
[  0] 0x000000010b566214 /Applications/MATLAB_R2021b.app/bin/maci64/libmwfl.dylib+00016916 _ZN10foundation4core4diag15stacktrace_base7captureERKNS1_14thread_contextEm+00000052
[  1] 0x000000010b56b9fa /Applications/MATLAB_R2021b.app/bin/maci64/libmwfl.dylib+00039418 _ZN10foundation4core4test17terminate_handledERKNSt3__112basic_stringIcNS2_11char_traitsIcEENS2_9allocatorIcEEEE+00011402
[  2] 0x000000010b568a75 /Applications/MATLAB_R2021b.app/bin/maci64/libmwfl.dylib+00027253 _ZN10foundation4core4diag13terminate_logEPKcPK17__darwin_ucontext+00000149
[  3] 0x000000011653329b /Applications/MATLAB_R2021b.app/bin/maci64/libmwmcr.dylib+00557723 _Z19mnPrintErrorMessageRKNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEE+00013355
[  4] 0x0000000116530c92 /Applications/MATLAB_R2021b.app/bin/maci64/libmwmcr.dylib+00547986 _Z19mnPrintErrorMessageRKNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEE+00003618
[  5] 0x000000011652e5c8 /Applications/MATLAB_R2021b.app/bin/maci64/libmwmcr.dylib+00538056 mnFatalSignalHandler+00000152
[  6] 0x00007ff80482537d           /usr/lib/system/libsystem_platform.dylib+00013181 _sigtramp+00000029
[  7] 0x0000000306671670                                   <unknown-module>+00000000
[  8] 0x000000020ead4553 /Applications/OpenSim 4.5/OpenSim 4.5.app/Contents/Resources/opensim/sdk/lib/libosimCommon.dylib+00361811 _ZNK7OpenSim9Component26initComponentTreeTraversalERKS0_+00000531
[  9] 0x000000020ead4553 /Applications/OpenSim 4.5/OpenSim 4.5.app/Contents/Resources/opensim/sdk/lib/libosimCommon.dylib+00361811 _ZNK7OpenSim9Component26initComponentTreeTraversalERKS0_+00000531
[ 10] 0x000000020ead4553 /Applications/OpenSim 4.5/OpenSim 4.5.app/Contents/Resources/opensim/sdk/lib/libosimCommon.dylib+00361811 _ZNK7OpenSim9Component26initComponentTreeTraversalERKS0_+00000531
[ 11] 0x000000020ead4553 /Applications/OpenSim 4.5/OpenSim 4.5.app/Contents/Resources/opensim/sdk/lib/libosimCommon.dylib+00361811 _ZNK7OpenSim9Component26initComponentTreeTraversalERKS0_+00000531
[ 12] 0x000000020a72d4d6 /Applications/OpenSim 4.5/OpenSim 4.5.app/Contents/Resources/opensim/sdk/lib/libosimSimulation.dylib+02041046 _ZN7OpenSim5Model19createMultibodyTreeEv+00000662
[ 13] 0x000000020a72e60e /Applications/OpenSim 4.5/OpenSim 4.5.app/Contents/Resources/opensim/sdk/lib/libosimSimulation.dylib+02045454 _ZN7OpenSim5Model20extendConnectToModelERS0_+00000158
[ 14] 0x000000020eac6dc8 /Applications/OpenSim 4.5/OpenSim 4.5.app/Contents/Resources/opensim/sdk/lib/libosimCommon.dylib+00306632 _ZN7OpenSim9Component19finalizeConnectionsERS0_+00000104
[ 15] 0x0000000220a22c58 /Applications/OpenSim 4.5/OpenSim 4.5.app/Contents/Resources/opensim/sdk/lib/libosimJavaJNI.dylib+03861592 Java_org_opensim_modeling_opensimSimulationJNI_Model_1finalizeConnections_1_1SWIG_10+00000024
[ 16] 0x0000000144dac0c7                                   <unknown-module>+00000000
[ 17] 0x0000000144d9c2bd                                   <unknown-module>+00000000

[aymanhab]

@carmichaelong Here's the fundamental problem and why this shouldn't be allowed.

1. When you get a reference to set (property) and modify it, the rest of the code doesn't know that the set has been modified, accordingly the flag to trigger recreation the graph traversal of components from Properties is never turned on and you end up with a crash traversing to the deleted component. A possible worksround is to call initSystem instead of finalizeConnections.
2. C++ Shouldn't allow removal altogether due to const correctness (which is lost in scripting) but there could be a loop hole. The code above doesn't use a reference to the WrapObjectSet but makes a copy which can safely be removed from, but who cares 😃
3. Removing the 'remove' 😄 from the set loses functionality since there's no other way to remove and not all sets are used as model components.
4. A possible fix is to make the set<ModelComponents> handle the removal differently by throwing an exception that "Removal is not supported" as in reality we can't chase down references to components in the graph. Alternatively we can set the flag that the model needs rebuilding and let users hit problems downstream.

Please let me know what you think.