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 bugs in Octave module loading #47

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGES.current
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ See the RELEASENOTES file for a summary of changes in each release.
Version 2.0.10 (in progress)
============================

2013-05-09: kwwette
[Octave] Fix bugs in Octave module loading:
- fix a memory leak in setting of global variables
- install functions only once, to speed up module loads

2013-04-28: gjanssens
[Guile] Updates in guile module:
- Add support for guile 2.0
Expand Down
11 changes: 1 addition & 10 deletions Lib/octave/octrun.swg
Original file line number Diff line number Diff line change
Expand Up @@ -1289,16 +1289,7 @@ SWIGRUNTIMEINLINE octave_value SWIG_Octave_GetGlobalValue(std::string name) {
}

SWIGRUNTIME void SWIG_Octave_SetGlobalValue(std::string name, const octave_value& value) {
// It is critical that a newly-allocated octave_value is passed to set_global_value(),
// since it and the Octave symbol table take references to the values assigned to it.
// If we were to pass a reference to 'value' to set_global_value(), then the Octave
// symbol table would hold a reference to a variable owned by the SWIG .oct module.
// Both will think that they own the reference (since the .oct module is dynamically
// loaded, it appears to have its own C++ runtime), and so they will both try to
// de-allocate the octave_value on exit, resulting in a double-free or seg-fault.
// This is prevented by giving Octave its own heap-allocated copy of 'value'.
octave_value *pov = new octave_value(value);
set_global_value(name, *pov);
set_global_value(name, value);
}

SWIGRUNTIME void SWIG_Octave_LinkGlobalValue(std::string name) {
Expand Down
18 changes: 10 additions & 8 deletions Lib/octave/octruntime.swg
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,15 @@ DEFUN_DLD( SWIG_name, args, nargout, SWIG_name_usage ) {

SWIG_InstallOps(octave_swig_ref::static_type_id());

octave_swig_type::swig_member_const_iterator mb;
for (mb = module_ns->swig_members_begin(); mb != module_ns->swig_members_end(); ++mb) {
if (mb->second.first && mb->second.first->method) {
if (!SWIG_Octave_InstallFunction(me, mb->first)) {
return octave_value_list();
}
}
}

#if OCTAVE_API_VERSION_NUMBER < 37
mlock(me->name());
#else
Expand All @@ -296,16 +305,9 @@ DEFUN_DLD( SWIG_name, args, nargout, SWIG_name_usage ) {

}

octave_function *me = octave_call_stack::current();

octave_swig_type::swig_member_const_iterator mb;
for (mb = module_ns->swig_members_begin(); mb != module_ns->swig_members_end(); ++mb) {
if (mb->second.first && mb->second.first->method) {
if (!SWIG_Octave_InstallFunction(me, mb->first)) {
return octave_value_list();
}
}
else if (mb->second.second.is_defined()) {
if (mb->second.second.is_defined()) {
SWIG_Octave_SetGlobalValue(mb->first, mb->second.second);
SWIG_Octave_LinkGlobalValue(mb->first);
}
Expand Down