[issue43946] unpickling a subclass of list fails when it implements its own extend method

2021-04-27 Thread Gregory P. Smith


Gregory P. Smith  added the comment:

Thanks, that makes sense.  And it at least less ugly than the __new__ hack I 
was thinking of as a workaround.  though similar in spirit.

presumably self.append should get the same treatment for completeness given the 
PEP 307 text.

I didn't notice a place in the Python pickle docs that mentioned this specific 
thing about listitems and the need for append/extend.  I didn't do a thorough 
read though, maybe I overlooked something?  If not, it'd be worth figuring out 
how to get this uninitialized class calling of append/extend/__setitem__ for 
list and dict objects detailed in the main docs rather than off in the PEP.

It's semi-mentioned 
https://docs.python.org/3/library/pickle.html#object.__reduce__ here which is 
what the PEP added I suppose, but given this code has no custom __reduce__, we 
need to explicitly mention that list and dict subclasses supporting 
pickling/copying may need to be prepared to handle this situation.  With a 
versionchanged:: 3.7 noting that it now always applies to list subclasses 
without their own __reduce__.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue43946] unpickling a subclass of list fails when it implements its own extend method

2021-04-26 Thread Serhiy Storchaka


Serhiy Storchaka  added the comment:

>From PEP 307:

listitemsOptional, and new in this PEP.
 If this is not None, it should be an iterator (not a
 sequence!) yielding successive list items.  These list
 items will be pickled, and appended to the object using
 either obj.append(item) or obj.extend(list_of_items).
 This is primarily used for list subclasses, but may
 be used by other classes as long as they have append()
 and extend() methods with the appropriate signature.
 (Whether append() or extend() is used depends on which
 pickle protocol version is used as well as the number
 of items to append, so both must be supported.)

It says that obj.extend(list_of_items) should be called, not list.extend(obj, 
list_of_items). Changing it now can break classes which rely on overridden 
extend(). The special check in the C code is only for optimization, we can 
inline extend() if it was not overridden.

Unfortunately __setstate__() is called after extend(), so there is no way to 
set the validator in FieldList before it will be used in extend().

As a clever hack (if you do not want to do runtime check in every call of 
extend()) you can set

self.extend = lambda items: list.extend(self, items)

in the __new__ method and add

del self.extend

in __init__ and __setstate__.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue43946] unpickling a subclass of list fails when it implements its own extend method

2021-04-26 Thread Richard Levasseur


Richard Levasseur  added the comment:

Here's a self-contained repro:


```
import pickle

class MyList(list):
  def __init__(self, required, values):
self.required = required
super().__init__(values)

  def __getstate__(self):
return self.required

  def __setstate__(self, state):
self.required = state

  def extend(self, values):
assert self.required
super().extend(values)

mylist = MyList('foo', [1, 2])
pickled = pickle.dumps(mylist)
unpickled = pickle.loads(pickled)

print(mylist)

```

The above will raise an AttributeError when self.required is accessed in 
extend(). 

Oddly, defining a `__reduce__()` function that simply calls and returns 
`super().__reduce__()` seems to restore the previous behavior and things work 
again.

--
nosy: +richardlev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue43946] unpickling a subclass of list fails when it implements its own extend method

2021-04-26 Thread Gregory P. Smith


New submission from Gregory P. Smith :

The changes from https://bugs.python.org/issue29368 are causing a subclass of 
list trouble:

```
class FieldList(list):
...
def extend(...): ...
```

FieldList has its own extend and append methods that implement additional 
checks.  As it is a list subclass, the new `PyList_CheckExact()` from the 
afformentioned issue's 
https://github.com/python/cpython/commit/f89fdc29937139b55dd68587759cadb8468d0190
 where it used to be a `PyList_Check()` in 3.6 and earlier is causing the 
unpickling code to call the instance `.extend()` method instead of internally 
using `PyList_SetSlice()` at the C level.

Calling .extend() naturally fails at this point as __setstate__ hasn't yet been 
called so the FieldList instance is uninitialized.

Here's the code in question 
https://github.com/google/protorpc/blob/master/protorpc/messages.py#L1126

It used it work.  3.7 broke it.  What was unpicklable is now not.

To work around this logic would be needed in the extend (and append) methods to 
check if they're being called on an uninitialized instance.  That seems 
unreasonable.

_[credit to my colleague Richard L. for the diagnosis]_

--
components: Extension Modules, Library (Lib)
keywords: 3.7regression
messages: 392008
nosy: gregory.p.smith, serhiy.storchaka
priority: normal
severity: normal
stage: needs patch
status: open
title: unpickling a subclass of list fails when it implements its own extend 
method
type: behavior
versions: Python 3.10, Python 3.11, Python 3.7, Python 3.8, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com