osohq / oso

Oso is a batteries-included framework for building authorization in your application.
https://docs.osohq.com
Apache License 2.0
3.47k stars 174 forks source link

Oso initialization is not thread safe #1665

Open AstraLuma opened 1 year ago

AstraLuma commented 1 year ago

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 commented 1 year ago

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

AstraLuma commented 1 year ago

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

gneray commented 1 year ago

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

Routing internally either way.

AstraLuma commented 1 year ago

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