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

Prevent memory leak with repeated calls to .schema() #26

Closed
wants to merge 1 commit into from
Closed

Prevent memory leak with repeated calls to .schema() #26

wants to merge 1 commit into from

Conversation

sloria
Copy link
Contributor

@sloria sloria commented Nov 2, 2018

When marshmallow Schema classes are instantiated using
type(), they get added to marshmallow's internal
class registry.

To demonstrate:

from marshmallow import class_registry
from dataclasses import dataclass
from dataclasses_json import dataclass_json

@dataclass_json
@dataclass
class Person:
    name: str

lidatong = Person('lidatong')

lidatong.schema()

assert len(class_registry._registry['marshmallow.schema.PersonSchema']) == 1

lidatong.schema()

assert len(class_registry._registry['marshmallow.schema.PersonSchema']) == 2

This is a known issue (ref marshmallow-code/marshmallow#732).

Passing a blank name prevents the class from getting
added to the registry.

When marshmallow Schema classes are instantiated using
`type()`, they get added to marshmallow's internal
[class registry](https://github.com/marshmallow-code/marshmallow/blob/dev/marshmallow/class_registry.py).

To demonstrate:

```python
from marshmallow import class_registry
from dataclasses import dataclass
from dataclasses_json import dataclass_json

@dataclass_json
@DataClass
class Person:
    name: str

lidatong = Person('lidatong')

lidatong.schema()

assert len(class_registry._registry['marshmallow.schema.PersonSchema']) == 1

lidatong.schema()

assert len(class_registry._registry['marshmallow.schema.PersonSchema']) == 2
```

This is a known issue (ref marshmallow-code/marshmallow#732).

Passing a blank name prevents the class from getting
added to the registry.
@sloria
Copy link
Contributor Author

sloria commented Nov 2, 2018

Tests pass for me locally.

One side effect of this change is that it makes the repr and str for the schemas a bit weird:

from marshmallow import class_registry
from dataclasses import dataclass
from dataclasses_json import dataclass_json

@dataclass_json
@dataclass
class Person:
    name: str

lidatong = Person('lidatong')

print(lidatong.schema())  # <many=False>

But hopefully that's tolerable.

@sloria
Copy link
Contributor Author

sloria commented Nov 2, 2018

I ended up fixing the memory issue in the latest marshmallow release. So upgrading the required marshmallow (#27) should obviate this PR.

@lidatong
Copy link
Owner

lidatong commented Nov 3, 2018

Awesome! Thanks for detailing the issue and pushing through the marshmallow fix

@lidatong lidatong closed this Nov 3, 2018
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

Successfully merging this pull request may close these issues.

2 participants