Skip to content

Work around SWIG stdout/err redirection bug for the PulsarParametersPy class

Karl Wette requested to merge ANU-CGA/lalsuite:swig-redirect-stdouterr-PPPworkaround into master

Description

The SWIG stdout/err redirection code (!1827 (merged)) is currently not reentrant, i.e. it's not safe when a SWIG wrapper function ends up calling another SWIG wrapper function. This behaviour can however occur e.g. with destructor functions calling other destructor functions.

The linked list used by the LALPulsar PulsarParameters struct appears to trigger this behaviour. Exiting the linked list causes nested destructor calls (which don't actually destroy anything, but are part of SWIGLAL's parent memory tracking). These corrupt the non-reentrant stdout/err redirection code and cause a call to PulsarParametersPy::keys() to hang indefinitely. (This occurs in both IPython/Juypyter and regular Python command-line usage when stdout/err redirection is turned on, but appears not to be triggered inside scripts.)

This MR is a minimal workaround:

  • allow stdout/err redirection to be turned on/off by extending the lal.swig_redirect_standard_output_error() function to take a new state as input, and return the old state;
  • turn off stdout/err redirection inside loop in PulsarParametersPy::keys(), and restore to previous state afterwards.

API Changes and Justification

Backwards Compatible Changes

  • This change does not modify any class/function/struct/type definitions in a public C header file or any Python class/function definitions
  • This change adds new classes/functions/structs/types to a public C header file or Python module

Backwards Incompatible Changes

  • This change modifies an existing class/function/struct/type definition in a public C header file or Python module
  • This change removes an existing class/function/struct/type from a public C header file or Python module

The prototype for lal.swig_redirect_standard_output_error() has been modified to allow the stdout/err redirection state to be toggled.

Review Status

Fixes #582 (closed)

cc @matthew-pitkin @adam-mercer

Merge request reports