jazzband / django-push-notifications

Send push notifications to mobile devices through GCM or APNS in Django.
MIT License
2.24k stars 609 forks source link

Race condition in DeviceViewSetMixin.create() bypasses `UPDATE_ON_DUPLICATE_REG_ID` #596

Open ge0rg opened 3 years ago

ge0rg commented 3 years ago

I have an application with Android clients registering for FCM push. Due to a client bug, the Android client will send two identical POST requests to the GCMDeviceAuthorizedViewSet endpoint in parallel.

I have UPDATE_ON_DUPLICATE_REG_ID=True configured, so I would expect that one of the requests adds the device, and the other one updates it.

However, both requests end up adding the device to the table, resulting in two entries added, because there is no table locking between the instance query based on registration id in https://github.com/jazzband/django-push-notifications/blob/master/push_notifications/api/rest_framework.py#L142-L144 and the table insert in https://github.com/jazzband/django-push-notifications/blob/master/push_notifications/api/rest_framework.py#L156

This can be worked around by setting UNIQUE_REG_ID=True, but please see #595.

A proper solution would be to use update_or_create, which does the proper locking apparently.

Andrew-Chen-Wang commented 3 years ago

@ge0rg PR welcome!