Skip to content

SIGSEGV on shutdown of tests #1830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
potterphx opened this issue Jun 22, 2022 · 8 comments
Closed

SIGSEGV on shutdown of tests #1830

potterphx opened this issue Jun 22, 2022 · 8 comments

Comments

@potterphx
Copy link

Environment

  • Pythonnet version: 3.0.0.a2
  • Python version: 3.10.4 (locally, 3.8 through 3.10 in GitHub actions)
  • Operating System: Ubuntu 22.04 (works find under Windows)
  • .NET Runtime:

Details

  • Describe what you were trying to get done.

    In several new projects, unit tests running fine under Windows, but when run under Ubuntu would would fail. On examining closer found that the unit tests all ran and passed, but there would be a SIGSEGV fault during shutdown. In chasing down possibly causes of the issue, the most common trigger appeared to a having a variable at the module level of .Net Type or of a Python container type which contained a .Net type.

  • What commands did you run to trigger this issue? If you can provide a
    Minimal, Complete, and Verifiable example
    this will help us understand the issue.

    Starting with one of the simpler projects, reducing it down a simple test case that reproduces the code.

    • src/acme
      • dlls/netstandard2.0
        • There were other DLLs from the results of a csproj build, but to reproduce this error only Microsoft.Extensions.Logging.Abstractions.dll is needed.
      • python_logger_working.py
        """Implementation of PythonLogger."""
        import logging
        import os
        import sys
        import clr  # type: ignore
        
        sys.path.append(os.path.join(os.path.dirname(__file__), "dlls/netstandard2.0"))
        clr.AddReference(r"Microsoft.Extensions.Logging.Abstractions")
        
        from Microsoft.Extensions.Logging import LogLevel  # type: ignore
        
        
        def get_python_level(log_level: LogLevel) -> int:
            """Get the equivalent python logging level for a .NET LogLevel."""
            python_level: int = logging.CRITICAL
            if log_level == getattr(LogLevel, "None"):
                python_level = logging.NOTSET
            elif log_level == LogLevel.Trace:
                # Python has no trace level, so we return the level right below debug as trace.
                python_level = logging.DEBUG - 1
            elif log_level == LogLevel.Debug:
                python_level = logging.DEBUG
            elif log_level == LogLevel.Information:
                python_level = logging.INFO
            elif log_level == LogLevel.Warning:
                python_level = logging.WARN
            elif log_level == LogLevel.Error:
                python_level = logging.ERROR
            return python_level
    • tests
      • test_python_logger.py
        import logging
        import pytest
        
        import clr
        from src.acme.python_logger_working import get_python_level
        
        clr.AddReference(r"Microsoft.Extensions.Logging.Abstractions")
        from Microsoft.Extensions.Logging import LogLevel
        
        
        @pytest.mark.parametrize(
            "dotnet_level,expected_level",
            [
                pytest.param(LogLevel.Critical, logging.CRITICAL),
                pytest.param(LogLevel.Error, logging.ERROR),
                pytest.param(LogLevel.Warning, logging.WARNING),
                pytest.param(LogLevel.Information, logging.INFO),
                pytest.param(LogLevel.Debug, logging.DEBUG),
                pytest.param(LogLevel.Trace, logging.DEBUG - 1)
            ]
        )
        def test_get_python_level(dotnet_level, expected_level) -> None:
            result = get_python_level(dotnet_level)
        
            # Verify
            assert( result == expected_level)
  • Workarounds and addition scenarios follow in comments.

@potterphx
Copy link
Author

One can avoid the SIGSEGV fault by transforming the test_python_logger.py either:

  • Transform all of the test cases in pytest.mark.parmetrize to not use .Net types.
    @pytest.mark.parametrize(
        "int_dotnet_level,expected_level",
        [
            pytest.param(5, logging.CRITICAL),
            pytest.param(4, logging.ERROR),
            pytest.param(3, logging.WARNING),
            pytest.param(2, logging.INFO),
            pytest.param(1, logging.DEBUG),
            pytest.param(0, logging.DEBUG - 1)
        ]
    )
    def test_get_python_level(int_dotnet_level, expected_level) -> None:
        # Setup
        dotnet_level = LogLevel(int_dotnet_level)
    
        # SUT
        result = get_python_level(dotnet_level)
    
        # Verify
        assert( result == expected_level)
  • Do not use pytest.mark.parametrize and instead write multiple tests for each test case.
    clr.AddReference(r"Microsoft.Extensions.Logging.Abstractions")
    from Microsoft.Extensions.Logging import LogLevel
    
    def test_get_python_level_critical() -> None:
        _test_get_python_level(LogLevel.Critical, logging.CRITICAL)
    
    
    def test_get_python_level_error() -> None:
        _test_get_python_level(LogLevel.Error, logging.ERROR)
    
    # ...
    
    def _test_get_python_level(dotnet_level, expected_level) -> None:
        result = get_python_level(dotnet_level)
    
        # Verify
        assert( result == expected_level)

@potterphx
Copy link
Author

Rewriting src/python_logger.py so as to not use a chained-if, but to use a dictionary may cause the SIGSEGV fault:

__dotnet_to_python = {
    getattr(LogLevel, "None"): logging.NOTSET,
    # Python has no trace level, so we return the level right below debug as trace.
    LogLevel.Trace: logging.DEBUG - 1,
    LogLevel.Debug: logging.DEBUG,
    LogLevel.Information: logging.INFO,
    LogLevel.Warning: logging.WARN,
    LogLevel.Error: logging.ERROR,
}


def get_python_level(log_level: LogLevel) -> int:
    global __dotnet_to_python
    return __dotnet_to_python.get(log_level, logging.CRITICAL)

One can work around this issue by adding an atexit function to deallocate the lookup dictionary during shutdown.

def __at_exit():
    global __dotnet_to_python
    __dotnet_to_python = None


atexit.register(__at_exit)

@potterphx
Copy link
Author

Using the second modification from the first comment to avoid the issue in the test file, and the first modification from the second comment to have the issue be caused from the code, I then added to the test file a main so that the tests could be run without pytest.

if __name__ == '__main__':
    print("START")
    test_get_python_level_critical()
    test_get_python_level_error()
    test_get_python_level_warning()
    test_get_python_level_information()
    test_get_python_level_debug()
    test_get_python_level_trace()
    print("END")

Directly running this also generated the SIGSEGV fault, thus demonstrating that pytest is not a requirements to reproduce the issue.

@potterphx
Copy link
Author

potterphx commented Jun 22, 2022

As soon as I finished the above write-up I thought of a simpler case:

tests/test_simple.py:

'''Very simple test case.'''
import clr  # type: ignore
from System import Object

__bad_magic: Object = Object()

def test_something():
    assert True

@lostmsu
Copy link
Member

lostmsu commented Jun 22, 2022

3.0.0a2 is rather old. We had a similar (crash on exit) issue fixed after it was released. Can you try Installing from master instead?

@potterphx
Copy link
Author

"rather old", it is the latest alpha release. 😁 Sorry, sarcasm slipped out.

Updating to current latest master (3.0.0.dev1 43d1640), and all the cases documented above no longer generated a SIGSEGV. Thanks.

Any ETA on another alpha?

@lostmsu
Copy link
Member

lostmsu commented Jun 22, 2022

@filmor can you publish a3 on pypi? That was a nasty bug we fixed. And the latest (a2) is from Dec 2021.

@filmor
Copy link
Member

filmor commented Jun 23, 2022

I'll finish up #1817 today (at least structurally) and release it as rc1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants