I pushed a more thorough change inspired by

previously it was possible for this self.myfileobj to be left open if any of the constructor failed

Rather than the try/except wrapping, why not add a flag which is set toFalse at the beginning of the constructor andTrue at the end, in__del__ (which is what emits the resource warning), check that flag. If it's not fully constructed then don't emit the resource warning?

The I/O stack (GzipFile inherits from IOBase) inIOBase.__del__ always does the close call ifself.closed is False, so shouldn't need to manually add a special case here. GzipFile.closed is:

    @property    def closed(self):        return self.fileobj is None

(so if there is a fileobj set, close should always be called).

This wouldn't help with myfileobj closing

How about usingself.fileobj is not None as the "fully initialized" flag (already set to None always, only set to non-None at end of__init__), and checking aroundif self.filobj is None and self.myfileobj is not None in__del__?

It feels a lot like this is adding more cases / code around closing. Trying to find a way to get this change to be smaller / more minimal to reduce risks. IO stack construction, dealloc warnings, and close + destruction order is already really intricate and hard to understand in full.

We can't rely onself.myfileobj being closed in__del__ and we don't return an instance ofGzipFile for the caller to close soself.myfileobj needs to be closed at the end of__init__ if there's an exception: so we need the try/catch

Right but this is closing objects that are inaccessible to the caller

TheGzipFile /self is equally inaccessible, and cleaned up by its__del__ / finalizertp_finalize (ortp_dealloc). It's guaranteed__del__ is called and able to release any held resources (such asself.myfileobj) seePEP-442.

I'm okay adding a cleanup for the case whereself.fileobj is None and self.myfileobj is not None inside__del__. Matches the behavior of otherio objects and is a lot smaller diff.

This is not correct, you cannot rely on__del__ to be called when an exception is raised because the exception traceback holds a reference to self

That statement seems to contradictPEP 442 Predictability section to me...

Following this scheme, an object’s finalizer is always called exactly once, even if it was resurrected afterwards.
For CI objects, the order in which finalizers are called (step 2 above) is undefined.

Even if there is an exception traceback that keeps the reference to self, eventually that traceback will go out of scope and file will be closed / not leaked...

I'll defer to@vstinner and@serhiy-storchaka for approval. From my perspective, this is a lot more complex of a bugfix than required, didn't match stated description and arguments aroundself.myfileobj. Happy to do quick fixes, can produce a much smaller patch here if desired with less behavior change.