Be careful with .pop

I've been fighting with a weird bug in my app: if I'm sending a push notification to 2+ users, only the first one receives the notification.

The bug was that I .poped the message key from the data.

Here's how it works (pseudo-code):

def send(users: [User], data: dict):
    notifications = reduce(lambda acc, user: acc + [Notification(user, data)], users, [])
    db.bulk_insert(notifications)
    for notification in notifications:
        notification.send()

...

def Notification.send(self):
    message = self.data.pop('message') 
    Firebase.send(message, self.data)

The first notification will be sent and I thought that message would be popped out of self.data, but actually it would be popped out of the dict that self.data points to! That means that every Notification object that has been initialized in the send function using Notification(user, data) (and not Notification(user, data.copy())) would not have data['message'] anymore, leading to unexpected behavior.

If you really need to get rid of a key in a dict, you should .copy() it before mutating.
If you don't - just stick to .get().

Top comments (5)

rhymes • Mar 26 '19

Good catch Defman!

Keep in mind that copy() does not create a deep copy so if you modify the Notification objects... both references will point to the same Notification object that you modified :D

If you have memory constraints and you want to let the GC do its work you could investigate the usage of weakref:

A primary use for weak references is to implement caches or mappings holding large objects, where it’s desired that a large object not be kept alive solely because it appears in a cache or mapping.

Sergey Kislyakov • Mar 26 '19 • Edited

Keep in mind that copy() does not create a deep copy so if you modify the Notification objects... both references will point to the same Notification object that you modified :D

I don't quite understand that. Doesn't {"a": 1, "b": {"c": 2}}.copy() create a copy of a dict (and allocate it in the memory) that would pass assert id(Notification1.data) != id(Notification2.data) (so by modifying Notification1.data I would not affect Notification2.data)?

rhymes • Mar 26 '19 • Edited

copy() performs a shallow copy:

In [1]: inner_dict = {"c": 2}

In [2]: d = {"a": 1, "b": inner_dict}

In [3]: c_d = d.copy()

In [4]: inner_dict["x"] = 3

In [5]: inner_dict
Out[5]: {'c': 2, 'x': 3}

In [6]: d
Out[6]: {'a': 1, 'b': {'c': 2, 'x': 3}}

In [7]: c_d
Out[7]: {'a': 1, 'b': {'c': 2, 'x': 3}}

In [8]: id(d['b']), id(c_d['b'])
Out[8]: (4364454576, 4364454576)

as you can see both d and its copy c_d point to the same inner_dict which when modified it reflects on both the original and the copy.

TLDR; if any of the values of the dict that's about to be copied is a reference to an object (in this case another dictionary, in your case a Notification object) then the original and the copy will point to the same object.

Sergey Kislyakov • Mar 26 '19

I see. Thanks!

In my case, data does not hold a Notification object. It doesn't hold any references at all, actually. It's a simple dict that looks like this: {"message": "my message", "some_attr": 123}.