Resource leak by not closing the client
Closed this issue · 10 comments
The HTTP client is not explicitly closed, leading to possible leaks. The JAX-RS ClientBuilder
is dynamically resolved, which in my case uses RestEasy instead of Jersey. In that case it uses a finalizer
to log a warning that the client wasn't closed, which is a requirement of the API. It would also be nice if these were pooled.
These issues makes it a little uncomfortable to use this SDK in a production with a reasonable email volume.
* Client is the main entry point to the fluent API used to build and execute client
* requests in order to consume responses returned.
* <p/>
* Clients are heavy-weight objects that manage the client-side communication
* infrastructure. Initialization as well as disposal of a {@code Client} instance
* may be a rather expensive operation. It is therefore advised to construct only
* a small number of {@code Client} instances in the application. Client instances
* must be {@link #close() properly closed} before being disposed to avoid leaking
* resources.
mailgun/src/main/java/net/sargue/mailgun/Mail.java
Lines 143 to 158 in abd667c
You are right. I've been aware of this issue for a while but never faced a problem (low volume of emails). I always end up postponing the solution.
I guess it's time for me to find some time to think of a solution.
By the way, MR are accepted. ;-)
Thank you. I decided to write a client in Retrofit instead so that I could use reuse a singleton client with its connection pool, metrics, etc. So this isn't a pressing concern for me, at least.
Hi all, I am looking at this since I want to send email with mailgun and spring boot 1.5.9
- I have not choose a solution yet, @ben-manes will you share your module?
- What is the recommended way to send mailgun from java at this time?
Thanks for sharing!
@kopax,
I was using simpleJavaMail, which is a really excellent library. However, the SMTP endpoint is very flaky at scale and requires retries. Mailgun's support wasn't helpful except acknowledging that they do some nasty things in their load balancer to cycle IPs for defensive reasons. They said the HTTP api was faster and more robust, so I had to port over.
I wrote an Email
class similar to @sargue's MailBuilder
class to aggregate the contents into a multipart. This goes to a Retrofit interface, which is backed by a shared OkHttp client. Not much work thanks to this library and the api docs to clarify their endpoint.
Thanks, @ben-manes for sharing this. I have opened the file and see the whole part. It would be nice to have a GitHub repo for this as well (fork) but that's not a requirement. (I assume you are using the latest version of each class imported in your files).
My requirement are:
- text email
- HTML email
- attachment (being able to send PDF files)
I have not done much bulk mail sending so I can't tell if the work is already done here in your zip file, do you think it is a good starting point to start with your achieved work?
Thanks again,
My requirements are the same and not bulk mail, but it amounts to around 800k/month. This is for example a company sending an invoice to their customer.
I did not include the code using this client, which adds those to the Email
class, invokes the Mailgun.send
method, and handles all of the metrics / retrials / etc. You would use this as,
Email email = new Email()
.from(...).to(...)
.text("Hello World")
.attachment(filename, file, mediaType);
mailgun.send(domain, email);
Anyone is welcome to formalize into a reusable client, like @sargue's nice library. It was not much work on my part, Retrofit does all the heavy lifting and this is just an example of how quickly one can bootstrap a usage.
Apparently @sargue fixed the issue there. Thanks a lot for this.
Now that this is fixed, I wonder which one should I use, is there any difference ?
I think OSS has some advantages, @sargue do you have anytought on @ben-manes work ?
Thanks @sargue!
I started with @sargue's and would have been happy to use it. Likewise it was little effort to write my own, as I didn't want to create pressure for a fix and continued maintenance (OSS can be exhausting as a never ending responsibility of free labor). There is no technical reason to not use @sargue other than if you desire more API features where mine provides a better starting point thanks to Retrofit.
I haven't checked Ben's work and I don't know Retrofit so I really can't say. You should evaluate your needs and pick was best for you.