Do not assign new fp attribute when exiting context manager

[radarhere]

At the moment,

from PIL import Image
with Image.new("L", (1, 1)) as im:
  print(hasattr(im, "fp"))
print(hasattr(im, "fp"))

gives

False
True

im does not have an fp attribute inside the context manager, but once it leaves, it is set.

This PR keeps it unset.

[akx]

Might be easier to read if the outermost layer of the pyramid was negated?

Suggested change

	if hasattr(self, "fp"):
	if getattr(self, "_exclusive_fp", False):
	if getattr(self, "_fp", False):
	if self._fp != self.fp:
	self._fp.close()
	self._fp = DeferredError(ValueError("Operation on closed image"))
	if self.fp:
	self.fp.close()
	self.fp = None
	if not hasattr(self, "fp"):
	return
	if getattr(self, "_exclusive_fp", False):
	if getattr(self, "_fp", False):
	if self._fp != self.fp:
	self._fp.close()
	self._fp = DeferredError(ValueError("Operation on closed image"))
	if self.fp:
	self.fp.close()
	self.fp = None

[radarhere]

Easier to read in isolation, yes, but I kind of like the fact that this version mirrors close() -

Pillow/src/PIL/Image.py

Lines 530 to 565 in 5431b15

	def __exit__(self, *args):
	if hasattr(self, "fp"):
	if getattr(self, "_exclusive_fp", False):
	if getattr(self, "_fp", False):
	if self._fp != self.fp:
	self._fp.close()
	self._fp = DeferredError(ValueError("Operation on closed image"))
	if self.fp:
	self.fp.close()
	self.fp = None
	def close(self):
	"""
	Closes the file pointer, if possible.
	This operation will destroy the image core and release its memory.
	The image data will be unusable afterward.
	This function is required to close images that have multiple frames or
	have not had their file read and closed by the
	:py:meth:`~PIL.Image.Image.load` method. See :ref:`file-handling` for
	more information.
	"""
	if hasattr(self, "fp"):
	try:
	if getattr(self, "_fp", False):
	if self._fp != self.fp:
	self._fp.close()
	self._fp = DeferredError(ValueError("Operation on closed image"))
	if self.fp:
	self.fp.close()
	self.fp = None
	except Exception as msg:
	logger.debug("Error closing: %s", msg)
	if getattr(self, "map", None):

[akx]

Should that code be extracted from close() then and reused in both of these places? _close_fp()?

[radarhere]

Ok, I've pushed a commit.

[akx]

Out of curiosity: does exclusive_fp mean, like I think it does, "we own this fp because we opened it"? If so, does it make sense for .close() to also close FPs that aren't "owned" by this instance?

[radarhere]

Out of curiosity: does exclusive_fp mean, like I think it does, "we own this fp because we opened it"?

Yes

Pillow/src/PIL/Image.py

Lines 3250 to 3258 in 697c24b

	if filename:
	fp = builtins.open(filename, "rb")
	exclusive_fp = True
	try:
	fp.seek(0)
	except (AttributeError, io.UnsupportedOperation):
	fp = io.BytesIO(fp.read())
	exclusive_fp = True

If so, does it make sense for .close() to also close FPs that aren't "owned" by this instance?

See #5309