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

Fix memory leak with Coordinate::_lockFunction. #510

Merged
merged 5 commits into from
Jul 14, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ before_install:
install:
- mkdir ~/opensim-core-build && cd ~/opensim-core-build
# Configure OpenSim.
- cmake $TRAVIS_BUILD_DIR -DBUILD_JAVA_WRAPPING=$BUILD_WRAPPING -DBUILD_PYTHON_WRAPPING=$BUILD_WRAPPING -DSWIG_EXECUTABLE=$HOME/swig/bin/swig -DSIMBODY_HOME=~/simbody-install -DCMAKE_CXX_FLAGS=-Werror -DCMAKE_INSTALL_PREFIX=$OPENSIM_HOME
- cmake $TRAVIS_BUILD_DIR -DBUILD_JAVA_WRAPPING=$BUILD_WRAPPING -DBUILD_PYTHON_WRAPPING=$BUILD_WRAPPING -DSWIG_EXECUTABLE=$HOME/swig/bin/swig -DSIMBODY_HOME=~/simbody-install -DCMAKE_CXX_FLAGS="-Werror -Wno-error=deprecated-declarations" -DCMAKE_INSTALL_PREFIX=$OPENSIM_HOME
# Build OpenSim.
# Build java and python C++ wrapper separately to avoid going over the memory limit.
- if [[ "$BUILD_WRAPPING" = "on" ]]; then make -j$NPROC osimTools osimJavaJNI PyWrap; fi
Expand Down
30 changes: 20 additions & 10 deletions OpenSim/Simulation/SimbodyEngine/Coordinate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
#include <OpenSim/Simulation/Model/Model.h>
#include <OpenSim/Simulation/SimbodyEngine/Joint.h>

#include <memory>

//=============================================================================
// STATICS
//=============================================================================
Expand Down Expand Up @@ -170,8 +172,6 @@ void Coordinate::extendFinalizeFromProperties()
cerr << "Default value = " << dv << " > max = " << get_range(1) << endl;
}
}
// Define the locked value for the constraint as a function
_lockFunction = new ModifiableConstant(get_default_value(), 1);

_lockedWarningGiven=false;

Expand All @@ -182,17 +182,27 @@ void Coordinate::extendAddToSystem(SimTK::MultibodySystem& system) const
{
Super::extendAddToSystem(system);

//create lock constraint automatically
// Make this modifiable temporarily so we can record information needed
// to later access our pieces of the SimTK::MultibodySystem. That info is
// const after the system has been built.
Coordinate* mutableThis = const_cast<Coordinate *>(this);

// Define the locked value for the constraint as a function.
// The PrescribedMotion will take ownership, but we'll keep a reference
// pointer here to allow for later modification.
std::unique_ptr<ModifiableConstant>
funcOwner(new ModifiableConstant(get_default_value(), 1));
mutableThis->_lockFunction = funcOwner.get();

// The underlying SimTK constraint
SimTK::Constraint::PrescribedMotion lock(system.updMatterSubsystem(),
_lockFunction.get(),
_bodyIndex,
SimTK::MobilizerQIndex(_mobilizerQIndex));
SimTK::Constraint::PrescribedMotion
lock(system.updMatterSubsystem(),
funcOwner.release(), // give up ownership
_bodyIndex,
SimTK::MobilizerQIndex(_mobilizerQIndex));

// Beyond the const Component get the index so we can access the SimTK::Constraint later
Coordinate* mutableThis = const_cast<Coordinate *>(this);
// Save the index so we can access the SimTK::Constraint later
mutableThis->_lockedConstraintIndex = lock.getConstraintIndex();
//mutableThis->_model->addModelComponent(mutableThis);

if(!getProperty_prescribed_function().empty()){
//create prescribed motion constraint automatically
Expand Down
9 changes: 5 additions & 4 deletions OpenSim/Simulation/SimbodyEngine/Coordinate.h
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,13 @@ OpenSim_DECLARE_CONCRETE_OBJECT(Coordinate, ModelComponent);
/* MobilizedBodyIndex of the body which this coordinate serves. */
SimTK::MobilizedBodyIndex _bodyIndex;

/* Mobilizer Q (i.e. genearlized coordinate) index for this Coordinate. */
/* Mobilizer Q (i.e. generalized coordinate) index for this Coordinate. */
SimTK::MobilizerQIndex _mobilizerQIndex;

/* Keep a reference to the SimTK function owned by the PrescribedMotion
Constraint, so we can change the value at which to lock the joint. */
SimTK::ReferencePtr<ModifiableConstant> _lockFunction;

/* Motion type (translational, rotational or combination). */
MotionType _motionType;

Expand All @@ -305,9 +309,6 @@ OpenSim_DECLARE_CONCRETE_OBJECT(Coordinate, ModelComponent);
/* The OpenSim::Joint that owns this coordinate. */
SimTK::ReferencePtr<const Joint> _joint;

/* SimTK function used to lock the joint at a fixed value */
mutable SimTK::ReferencePtr<ModifiableConstant> _lockFunction;

mutable bool _lockedWarningGiven;

// PRIVATE METHODS implementing the Component interface
Expand Down
13 changes: 8 additions & 5 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
# "/EHsc" is to "unwind semantics" to get error messages when using "/WX" (C4530).
# However, this was causing some weird behavior. Not treating warnings
# as errors for now.
#
### TEMPORARILY DISABLING WRAPPING (see lines with ###)

shallow_clone: true

Expand All @@ -31,10 +33,10 @@ nuget:
install:

## Use Chocolatey to install SWIG.
- choco -y install swig
###- choco -y install swig

## Install python-nose for python testing.
- pip install nose
###- pip install nose

## Simbody.
# Simbody's installation is pushed to our Appveyor NuGet account feed.
Expand All @@ -51,7 +53,8 @@ build_script:
- mkdir build
- cd build
# Configure.
- cmake .. -G"Visual Studio 12 2013 Win64" -DSIMBODY_HOME=C:\simbody -DCMAKE_INSTALL_PREFIX=%OPENSIM_HOME% -DBUILD_JAVA_WRAPPING=ON -DBUILD_PYTHON_WRAPPING=ON # TODO -DBUILD_SIMM_TRANSLATOR=ON
###- cmake .. -G"Visual Studio 12 2013 Win64" -DSIMBODY_HOME=C:\simbody -DCMAKE_INSTALL_PREFIX=%OPENSIM_HOME% -DBUILD_JAVA_WRAPPING=ON -DBUILD_PYTHON_WRAPPING=ON # TODO -DBUILD_SIMM_TRANSLATOR=ON
- cmake .. -G"Visual Studio 12 2013 Win64" -DSIMBODY_HOME=C:\simbody -DCMAKE_INSTALL_PREFIX=%OPENSIM_HOME% -DBUILD_JAVA_WRAPPING=OFF -DBUILD_PYTHON_WRAPPING=OFF
# Build.
- cmake --build . --target ALL_BUILD --config Release -- /maxcpucount:4 /verbosity:quiet

Expand All @@ -75,10 +78,10 @@ test_script:
- copy Applications\CMC\test\gait10dof18musc_subject01.osim %OPENSIM_HOME%\Models\Gait10dof18musc\gait10dof18musc.osim

# Move to the installed location of the python package.
- cd %OPENSIM_HOME%\sdk\python
###- cd %OPENSIM_HOME%\sdk\python

# Run python tests.
- nosetests -v
###- nosetests -v

after_test:
- ## On master branch, create NuGet package for OpenSim.
Expand Down