Skip to content

SWIG: disown Python/Octave objects assigned to viewed C arrays

Karl Wette requested to merge (removed):swig-multi-array-assignment-fix into master

Description

This fixes a bug in the SWIG wrappings, first reported by Reinhard, when assigning new objects to arrays in structs. It's best explained with an example:

mts = lalpulsar.CreateMultiLIGOTimeGPSVector(2)   # creates an array type, 'mts' owns its memory
ts0 = lalpulsar.CreateTimestampVector(3)          # creates a new element, 'ts0' owns its memory
mts.data[0] = ts0   # assigns 'ts0' to an element of array in 'mts', now both 'mts' and 'ts0' claim to own 'ts0's memory
del mts             # free 'mts' and its memory, including that of new element which 'ts0' claims to own
del ts0             # try to free 'ts0' and its memory, results in double free / segfault

The fix is for 'ts0' to disown the memory it points to when it is assigned to mts.data[0], so that now only 'mts' owns the memory.

Patches are:

  1. SFTfileIO: move XLALCreateMultiLIGOTimeGPSVector() function here: moves a functions into LALPulsar so that a test case can be written. (This function should be in LALPulsar in any case.)
  2. SWIGLALAlpha.i: print warning messages when setting nice/nasty error handlers: minor cosmetic fixes to error handlers so that they print warning messages re. what error handlers are in effect. (Important note: the only way to catch the failure above, in Python at least, is to change the XLAL/LAL error handlers to abort on error. A try ... except block won't work since del cannot raise exceptions.)
  3. SWIG: disown Python/Octave objects assigned to viewed C arrays: fixes the bug, add test cases in the SWIG tests in LALPulsar to check fix works.

API Changes and Justification

Backwards Compatible Changes

  • This change introduces no API changes
  • This change adds new API calls

Backwards Incompatible Changes

  • This change modifies an existing API
  • This change removes an existing API

Review Status

Fixes the test case reported by Reinhard (which is the same as that given above).

Edited by Adam Mercer

Merge request reports

Loading