-
Notifications
You must be signed in to change notification settings - Fork 40.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Leaking file descriptor / socket within DomainSocket tooling #32423
Comments
Thanks for getting in touch.
Are you referring to the project at https://github.com/docker-java/docker-java or another project? I see no indication that this
The code in the |
Ok, that was maybe a bit confusing. It seems they simply "copied" your code. I thought it would be better reaching you at first because your are the original author and would be affected also. The copy is e.g. here https://github.com/docker-java/docker-java/blob/dd1735831f12f787070b667d33ac1c4103630c95/docker-java-transport/src/main/java/com/github/dockerjava/transport/DomainSocket.java They (and me) dont use your specific code directly. If you think you are not affected for this or it is not relevant, we can close this. Don't get me wrong, I just thought you were interested in this one. :) |
Thank you for clarifying the copying of code vs direct use, that makes sense now. Spring Boot users shouldn't be affected since the code is only called by the Maven and Gradle plugins, and runs in the context of a Maven goal or Gradle task. It will not make its way into a user's application unless they choose to use it directly, which is unlikely since the class has little value on its own and the code in Spring Boot that calls it is package-private. Thanks again for the high-quality issue with the demo project, but this should be reported to the |
After some discussion, we've decided this is worth looking into for the Boot plugins. Though it won't affect user applications, there is a chance that the Gradle or Maven daemon processes could be long-running and encounter this condition. |
This is the result of a long journey searching for a very strange leaking issue. I hope I have put everything together again, found the right address. Otherwise please ask for more details.
So, let's start of my finding: Probably this affects the component "buildpack" (unsure how you named this), but not the actual "main" framework/boot library system.
As far as I can say, I would say that the call chain of
results into a resource leak under the condition the
path
does not exist. I can see there is native code involved, because the exception happens in the connect method:Yes, the package differs but it is only shaded (as far as I can say it is identical).
I have a demo provided at this repository.
Please have a look at the open files counter (which is based on the same as FileDescriptorMetrics).
I have found this class while searching for a leak when using the library docker-java which have included these classes (credits of Phil Webb left, so I found the origin here). Because you (the project) are the actual author, I guess this issue is better addresses here. If the finding is correct and/or we find a solution, I will follow-up for the required changes to them.
The symptom is the system (linux) reports for the java process with
lsof -p <pid> | grep STREAM
multiple entriesA couple of them are fine, but this list grows only in one of our applications. I took some time isolating the actual culprit, at the end speaking for this context the application does (reduced): call each minute
DomainSocket.get("/var/lib/docker.sock")
, but this socket does not exist on this system. (For sake of simplicity I have reduced the dynamic aspect here.)On every "call", the list grows for more entries (which means more open files). The counter "open files" in the java runtime grows as well. The (linux) system reports this files as sockets without any meta information except the mapped pid.
I am not sure whether the file descriptor should stay (I would say no, because the exception is thrown) or the call must be guarded with "file exist" check. However, I guess this should be part of at least the DomainSocket class?
The text was updated successfully, but these errors were encountered: