C++ macro in scope of if statement not compiling

Viewed 818

I have some code which is supposed to be a thread-safe python/c++ api. I am using the macros Py_BEGIN_ALLOW_THREADS and Py_END_ALLOW_THREADS, which expand to create save thread state and create a lock. I am releasing the lock just before method exit; once inside of if statement scope, and once at method scope.

Why does this not compile? It generates the error: error: _save was not declared in this scope at the second Py_END_ALLOW_THREADS macro.

uint8_t SerialBuffer::push_msg() {

#if defined (UBUNTU)
  Py_BEGIN_ALLOW_THREADS
#endif

  if (_type == ARRAY) {
      // array access
  } else if (_type == PRIORITY_QUEUE) {
      // queue access
  } else {

    // Placing the return statement in the preprocessor directive
    // has no effect.
#if defined (UBUNTU)
    Py_END_ALLOW_THREADS
#endif

    return FAIL;
  }

#if defined (UBUNTU)
  Py_END_ALLOW_THREADS
#endif

  return SUCCESS;
}

I have also tried putting the return statement inside of the #if directive scope, and this generates the same error. However, this works:

uint8_t SerialBuffer::push_msg() {

#if defined (UBUNTU)
  Py_BEGIN_ALLOW_THREADS
#endif

  if (_type == ARRAY) {
      // array access
  } else if (_type == PRIORITY_QUEUE) {
      // queue access
  } else {
    // NOTE lack of #if directive here.
    // Even though if this code executes the code below will not.
    // Seems like a relatively simple problem for lambda calculus, no?
    return FAIL;
  }

#if defined (UBUNTU)
  Py_END_ALLOW_THREADS
#endif

  return SUCCESS;
}

Edit: I'm aware that the second example does not do thread clean-up; however, it compiles.

Edit2: Py_BEGIN_ALLOW_THREADS expands to { PyThreadState *_save; _save = PyEval_SaveThread();

Py_END_ALLOW_THREADS expands to PyEval_RestoreThread(_save); } NOTE the scoping braces prepending BEGIN and appending END. Why is it the logical choice for the macro expansion to include scoping?

2 Answers

From [Python.Docs]: Initialization, Finalization, and Threads - Py_BEGIN_ALLOW_THREADS (emphasis is mine):

This macro expands to { PyThreadState *_save; _save = PyEval_SaveThread();. Note that it contains an opening brace; it must be matched with a following Py_END_ALLOW_THREADS macro. See above for further discussion of this macro.

So, the answer to the compile error is pretty clear:

  • After preprocessing, the 2nd Py_END_ALLOW_THREADS produces invalid code (and the enclosing in #if defined (UBUNTU) is irrelevant, as it will never work when UBUNTU is defined, and it will always work when it isn't):

    • Referencing (inexistent) "_save"
    • An extra closing brace ("}")

The common usecase for the 2 macros is also exemplified int he above page:

PyThreadState *_save;
_save = PyEval_SaveThread();

... Do some blocking I/O operation ...

PyEval_RestoreThread(_save);

Why it was designed this way (to include scoping)? Probably to fail when used like you did, because that can cause hard to find errors (your example is pretty simple, but in a more complex piece code with many branches requiring just as many Py_END_ALLOW_THREADS, imagine what would mean to miss one, or to call it twice).

In order to fix your problem, you must redesign your code to either:

  • Not directly return on failure, but flag it, return (in one place) at the end, and place the Py_END_ALLOW_THREADS, just before returning
  • Call the macro pair inside each (or where required) of the if branches (ARRAY, PRIORITY_QUEUE, ...)
  • Use (the dreaded) goto
Related