Python 3.11 support

[edwardalee]

This is an attempt to get the python target to work with Python 11. It gets past the segfault, but when it imports the generated C module, Python for some reason fails to recognize the result as an extension module. For example, the ActionDelay.lf test gives this error message:

SystemError: initialization of LinguaFrancaActionDelay did not return an extension module

[lhstrh]

Shouldn't the CI configuration in this branch also be changed to work with Python 11?

[edwardalee]

Actually, I would like to make sure the branch works with Python 3.10. It does not yet work with Python 3.11.

[lhstrh]

Actually, I would like to make sure the branch works with Python 3.10. It does not yet work with Python 3.11.

I updated the CI config to that effect in 3bc9562...

[lhstrh]

From what I understood from @cmnrd, this PR provides fixes for a problem that shouldn't exist in the first place (or, rather, should be solved in a different way). He implied that we are not incompatible with Python 11 as is, and that the segmentation fault occurs due to building using with one version of Python and then running with another.

[lhstrh]

I'm referring to #1458 (comment).

[lhstrh]

I may be wrong about this, but let me defer to @cmnrd for the review of this PR.

[cmnrd]

This PR makes Python 3.11 required. Is this really what we want? I think we should aim at continuing the Python 3.10 support in order to also support running on Debian and the latest Ubuntu LTS release. This would mean, however, that we should also run tests for both Python versions.

[cmnrd]

From what I understood from @cmnrd, this PR provides fixes for a problem that shouldn't exist in the first place (or, rather, should be solved in a different way). He implied that we are not incompatible with Python 11 as is, and that the segmentation fault occurs due to building using with one version of Python and then running with another.

I think there is some confusion about the python errors, and I am not sure what this PR fixes. On my Linux machine with Python 3.11 things seem to run fine without any errors even on master. However, given that earlier CI runs in this PR actually failed, it looks like there are other error than the segmentation fault caused by incompatible Python runtimes.

[cmnrd]

I updated this branch today (merged in master, also updated the submodule and merged in reactor-c main) and extended CI so that we test both Python 3.10 and 3.11. However, now the tests fail again and I don't know what is going on. Must be some change that happened in the meantime in LF or reactor-c.

[lhstrh]

@jackykwok2024, could you please take a look at this?

[jackyk02]

I have consistently reproduced the error using the BanksCount3ModesSimple.lf file on my Linux machine. The code generated runs as expected with Python 3.10. However, it encounters a segmentation fault when executed using Python 3.11, as outlined below:

DEBUG: Invoking reaction function _countercycle.reaction_function_2
ERROR: FATAL: Failed to invoke reaction _countercycle.reaction_function_2
Traceback (most recent call last):
  File "/mnt/c/Users/jacky/Desktop/LF/python311/src-gen/BanksCount3ModesSimple/BanksCount3ModesSimple.py", line 74, in reaction_function_2
    count.set(3)
TypeError: Could not set objects.
Segmentation fault

Upon comparing the output logs between Python 3.10 and Python 3.11, I noticed a significant discrepancy in the reference count for the val within the py_port_set function.

Output from Python 3.11: DEBUG: Setting value 0xaa5f28 with reference count 1000000161
Output from Python 3.10: DEBUG: Setting value 0x7f53793c8130 with reference count 98

As a result, my conjecture is that this issue arises from a deep recursion in Count3Modes.lf.

reactor CounterCycle {
  input next
  output count

  initial mode One {
    reaction(next) -> count, reset(Two) {=
      count.set(1)
      Two.set()
    =}
  }

  mode Two {
    reaction(next) -> count, reset(Three) {=
      count.set(2)
      Three.set()
    =}
  }

  mode Three {
    reaction(next) -> count, reset(One) {=
      count.set(3)
      One.set()
    =}
  }
} 

According to the official documentation of Python 3.11, there is an optimization update for recursive functions and the C stack in Python to Python calls is removed (python/cpython#89419) However, I'm uncertain whether this is the underlying issue.

Additionally, here is the GDB backtrace:

[lhstrh]

Thanks, @jackykwok2024, that's a really good find! I agree that it seems plausible that this optimization is to blame... Seems hard to fix if true. Do you have any ideas?

[edwardalee]

I don't see any recursion in Count3Modes.lf, so it's not clear how this optimization could be related. ???

[lhstrh]

I figured there may be recursion involved with the mode transitions or set implementation, but I could well be wrong...

[jackyk02]

Sorry for the confusion in my previous comments. My initial conjecture was that the inflated reference count might have been due to recursion involved with the set implementation. However, after a more detailed review of the changes for Python 3.11, as indicated in the PRs, the issue seems to be different.

In Python 3.10 and earlier, the function sys.getrefcount() returns the reference count of an object, indicating the number of variables or data structures that reference it. However, Python 3.11 introduces a modification for statically allocated interpreter states, and certain objects like small integers are now marked as "immortal."

These immortal objects have an artificially inflated reference count, for example, 1000000161 instead of a typical smaller number. This is what we are observing as the reference count in the py_port_set function.

This issue seems to be unrelated to the segfault but could lead to confusion for developers during the debugging process…

[edwardalee]

A minor issue is that with python 3.11, we get a deprecation warning that should be easy to fix:

pythontarget.c:600:9: warning: 'Py_SetPath' is deprecated [-Wdeprecated-declarations]
        Py_SetPath(wcwd);

This will require a fix in reactor-c.

[lhstrh]

This looks good to me. I noticed that we're increasing the testing burden by testing both Python 3.10 and 3.11. What should be the policy going forward? What range of Python version do we plan to support?

[cmnrd]

Have we made progress in figuring out why the tests do not pass?

I noticed that we're increasing the testing burden by testing both Python 3.10 and 3.11. What should be the policy going forward? What range of Python version do we plan to support?

It is up to us to decide which Python versions to support. With Mocasin we aim to support the 3 latest versions. I think we should aim to support at least the latest version, as well as the version used by the latest LTS Ubuntu (and Debian) releases (which is 3.10 at the moment).

[cmnrd]

Since Python 3.12 was released recently, we should actually also add it to the matrix.

[lhstrh]

I noticed that this PR has stalled. Is there any ambition to resurrect this effort?

[edwardalee]

I have no bandwidth to resurrect this, but we have a bigger problem, which is that CMake should not be choosing the Python version. We need a target property or some such. See #2298.