-
Notifications
You must be signed in to change notification settings - Fork 101
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
Multipart performs blocking call in every instantiation #699
Comments
Related: #101 |
Hi @jonathannaguin, where is the blocking operation thing in FactoryFinder?, I cannot find it. |
@jbescos I assumed that the blocking was from Class::forName, ServiceLoader::load, etc. My initial understanding of the fix is to follow advice from ServiceProvider "... Users are recommended to cache the result of this method." I don't think we are doing this everywhere but, I'm not sure if it is intended or not. |
The super class Multipart has a protected instance field that most likely causing this issue. |
That's right @jmehrens , https://github.com/jakartaee/mail-api/blob/master/api/src/main/java/jakarta/mail/Multipart.java#L69 will instantiate a StreamProvider (https://github.com/jakartaee/mail-api/blob/master/api/src/main/java/jakarta/mail/util/StreamProvider.java#L195) which will use FactoryFinder to find and instantiate given provider. I was able to workaround some of this by setting some system properties so the Factory finds the |
I think that field should be deprecated and removed when move to next major release. If we added a protected method instead of a field we could do searching of the provider of parent part which should have a session and or other child parts. That said, the easy solution is to create a private static final field and use that to assign the protected instance field. I just have not done the research to figure out if we lose anything by not running this code for every instance. |
The reason we depend on the StreamProvider is because we need an implementation of I remember @lukasj and me were discussing this when we were splitting mail-api and angus-mail. There was a dependency in the API with the implementation, and we had to solve that introducing the Is it really worth this overhead of trying to find a StreamProvider somewhere to obtain those instances of only two interfaces?. In my opinion, it is better idea to have those implementations of LineOutputStream/LineInputStream within the Mail-API. In case someone is able to create a more efficient LineOutputStream/LineInputStream implementation, we can just include it here. CC @lukasj |
That makes sense. I lean towards always getting the streamprovider from the session as that is a good spot to cache the lookup. In this case I think we can:
In any case, we would have to consider removing the instance field at some point to gain the benefit of this approach. I think this is a lower priority ticket. |
The problem of using the default session is that its SteamProvider is static. I imagine 2 different wep-apps running in Tomcat, one with angus-mail and the other with other implementation of SteamProvider. This second app could use the angus-mail StreamProvider loaded by the first app, and I think that is undesired. If it does not find the SteamProvider from its parents, I think it needs to create a new instance of it. Or we can just remove SteamProvider as I said before. |
Signed-off-by: Jorge Bescos Gascon <[email protected]>
Describe the bug
When a
Multipart
object is created, the fieldstreamProvider
is also instantiated which relies ultimately on FactoryFinder#find (https://github.com/jakartaee/mail-api/blob/master/api/src/main/java/jakarta/mail/util/FactoryFinder.java#L35) that performs blocking operations.To Reproduce
Create a new MimeMultipart object, for example
on a project with https://github.com/reactor/BlockHound installed.
Expected behavior
Either the FactoryFinder needs to be rewritten to not use non-blocking operations or
Multipart
could use a static reference so only it's blocked at boot up.The text was updated successfully, but these errors were encountered: