C++ macro in scope of if statement not compiling

Question

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?

Answer 1

The preprocessor expands the macro, Py_BEGIN_ALLOW_THREADS, into code that creates a local object named _save.

The preprocessor expands the macro, Py_END_ALLOW_THREADS, into code that uses _save to do thread clean-up tasks.

If you put Py_BEGIN_ALLOW_THREADS inside the else block, the code created by Py_END_ALLOW_THREADS can't see the local _save object, so you get an error message.

On a related topic, I recommend putting Py_BEGIN_ALLOW_THREADS and Py_END_ALLOW_THREADS where if the first one executes, then so does the second one. Your second version of the function will not do the thread clean-up tasks for Py_END_ALLOW_THREADS if you have an array type or a priority-queue type.

Try this:

uint8_t SerialBuffer::push_msg() {

#if defined (UBUNTU)
  Py_BEGIN_ALLOW_THREADS
#endif
  uint8_t response = FAIL;

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

#if defined (UBUNTU)
  Py_END_ALLOW_THREADS
#endif

  return response;
}

In this version, the default response is FAIL, so you don't even need the final else section. The other if statements only set the response to SUCCESS if all goes well.

Answer 2

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 2^ndPy_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