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

Possibility of race condition during import #1456

Closed
matthewhegarty opened this issue Jul 12, 2022 · 0 comments · Fixed by #1668
Closed

Possibility of race condition during import #1456

matthewhegarty opened this issue Jul 12, 2022 · 0 comments · Fixed by #1668
Labels

Comments

@matthewhegarty
Copy link
Contributor

Describe the bug

For any model which permits concurrent writes, the import process is vulnerable to a race condition.

  • The check for new or existing objects identifies that our row is 'new'
  • From another process, an object with the same unique id as the row is inserted before the call to save()
  • If there is a unique db constraint on the field, then the call to save will fail, otherwise a duplicate row will be inserted.

To Reproduce

  • Can be reproduced by inserting an object between get_or_init_instance() and save() - e.g. one could add a long sleep call in before_save_instance(), or simply create the object in before_save_instance().

Versions (please complete the following information):

  • Django Import Export: 2.8.1
  • Python 3.10
  • Django 4.0.5

Expected behavior

it would be ideal if the process was protected from this occurrence, such that you can assume that your object will be created safely.

Additional context

I have actually seen this issue in production. The workaround was to override save_instance() and add a call to create_or_update(), which is ok for my use case, but might not be for others, because the race condition still exists in terms of what gets updated. i.e. you could overwrite the other processes insert with an update.

Another idea might be to lock the table, but this could have negative performance implications. Maybe this should be documented and left as an option for users if it is important.

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

Successfully merging a pull request may close this issue.

1 participant