Skip to content
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

Oso initialization is not thread safe #1665

Open
AstraLuma opened this issue Jan 20, 2023 · 4 comments
Open

Oso initialization is not thread safe #1665

AstraLuma opened this issue Jan 20, 2023 · 4 comments

Comments

@AstraLuma
Copy link
Contributor

Because Reasons:tm: (just don't ask), I was able to get some Weird errors from Oso that I believe stems from multiple parallel initializations.

...
  File "/myapp/core.py", line 145, in _register_models
    oso.register_class(model, name=model.__name__)
  File "/usr/local/lib/python3.8/site-packages/polar/polar.py", line 253, in register_class
    self.host.register_mros()
  File "/usr/local/lib/python3.8/site-packages/polar/host.py", line 122, in register_mros
    for rec in self.distinct_user_types():
RuntimeError: dictionary changed size during iteration
polar.exceptions.PolarRuntimeError: Cannot load additional Polar code -- all Polar code must be loaded at the same time.
def get_oso():
    """
    Get the oso object
    """
    global OSO
    if OSO is None:
        OSO = Oso()

        app = get_base_app()
        # Add config as constants
        for name, value in app.config.items():
            OSO.register_constant(value, name)

        # Note: This is handled automatically if you use oso-sqlalchemy
        import myapp.models
        _register_models(OSO, myapp.models.Base)

        import myapp.osolib
        OSO.register_constant(myapp.osolib, 'lib')

        # https://github.com/osohq/oso/issues/1435
        OSO.load_resources([
            ('myapp', res)
            for res in importlib.resources.contents('myapp')
            if res.endswith('.polar')
        ])

    return OSO
@AstraLuma
Copy link
Contributor Author

I'm honestly expecting a "don't do that", but I figured i'd report the behavior upstream.

@AstraLuma
Copy link
Contributor Author

(to be clear, this pattern of initializing objects wasn't my idea, it was already established in the application when i took over.)

@gneray
Copy link
Contributor

gneray commented Jan 21, 2023

Thanks for bringing this to our attention! Confirming you're not blocked on this atm?

Routing internally either way.

@AstraLuma
Copy link
Contributor Author

Yeah, I fixed this in our case by doing some clever thread things so only one initialization can happen at once.

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

2 participants