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