FileDescriptor overrides finalize:

https://github.com/spring-projects/spring-boot/blob/9c9fbf400e11c40cd10b883cf986a9d4a65009e4/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/socket/FileDescriptor.java#L47-L50

I'm not sure that it's needed as DomainSocket – the only place where FileDescriptor is used – will close it when the socket is closed:

https://github.com/spring-projects/spring-boot/blob/9c9fbf400e11c40cd10b883cf986a9d4a65009e4/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/socket/DomainSocket.java#L113-L122

If the DomainSocket isn't being closed for some reason then we'd be leaking its input and output streams as the super.close() call is what causes them to be closed. I suspect we can just remove the override of finalize() completely. This'll help with 3.0 and the Java 17 baseline where finalize() has been deprecated. Flagging for team attention to see if I've missed something and it's needed.

Comment From: dreis2211

I wonder if this ticket could be extended to review the use of finalize() in general. There is another one in RestartClassLoader that seems not to be strictly needed: https://github.com/spring-projects/spring-boot/blob/1a505964ee5e1c68cb85e6cc2d99a7dbf9835677/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/classloader/RestartClassLoader.java#L191-L196

Comment From: philwebb

I think I was being super defensive adding the finalizer to FileDescriptor. I agree that we can drop it. I guess we don’t need the one in RestartClassLoader either. I probably added that one to check we didn’t have a restart memory leak.

Not sure if we should remove them in 2.4.x or not. Do they cause issues there?

Comment From: wilkinsona

All of the reasons given for deprecating finalize() in Java 9 apply to Java 8 and earlier as well so I don't think we should wait unless we really feel the risk in a maintenance release is unacceptably high.

AIUI, there's also a non-zero cost to overriding finalize(). Whenever an object that does so is created, a java.lang.ref.Finalizer is also created and, once the object's being GCed, it gets added to a reference queue from where it'll eventually be taken and finalize() called. While this is more of a problem when creating and GCing many such objects, I still think it's something that's worth avoiding.

I am in favour of removing the use of finalize() in 2.4.x as I think the risk is lower than the benefit.

Comment From: philwebb

Sounds good. 2.4.x it is then.