You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2022/03/28 04:51:38 UTC
[GitHub] [james-project] vttranlina opened a new pull request #940: JAMES-3724 - Fixing leak resource for MemoryMailRepository
vttranlina opened a new pull request #940:
URL: https://github.com/apache/james-project/pull/940
Fixing: Resource leaked
```
11:48:03.733 [ERROR] o.a.j.l.a.Disposable$LeakAwareFinalizer - Leak detected! Resource org.apache.james.server.core.MimeMessageInputStreamSource$Resource@66bd8880 was not released before its referent was garbage-collected.
This resource was instanced at:
org.apache.james.server.core.MimeMessageInputStreamSource#create:124
org.apache.james.server.core.MimeMessageWrapper#<init>:156
org.apache.james.server.core.MailImpl#setMessage:505
com.github.fge.lambdas.consumers.ConsumerChainer#lambda$sneakyThrow$9:73
java.util.Optional#ifPresent:183
org.apache.james.server.core.MailImpl$Builder#build:279
org.apache.james.server.core.MailImpl#duplicate:99
org.apache.james.server.core.MailImpl#duplicate:810
org.apache.james.mailrepository.memory.MemoryMailRepository#cloneMail:87
org.apache.james.mailrepository.memory.MemoryMailRepository#store:44
org.apache.james.transport.mailets.ToRepository#service:79
org.apache.james.mailetcontainer.impl.ProcessorImpl#process:73
com.github.fge.lambdas.consumers.ConsumerChainer#lambda$sneakyThrow$9:73
java.util.stream.ForEachOps$ForEachOp$OfRef#accept:183
java.util.stream.ReferencePipeline$2$1#accept:177
java.util.Collections$2#tryAdvance:4747
java.util.Collections$2#forEachRemaining:4755
java.util.stream.AbstractPipeline#copyInto:484
java.util.stream.AbstractPipeline#wrapAndCopyInto:474
java.util.stream.ForEachOps$ForEachOp#evaluateSequential:150
java.util.stream.ForEachOps$ForEachOp$OfRef#evaluateSequential:173
java.util.stream.AbstractPipeline#evaluate:234
java.util.stream.ReferencePipeline#forEach:497
org.apache.james.mailetcontainer.impl.MailetProcessorImpl#executeProcessingStep:162
org.apache.james.mailetcontainer.impl.MailetProcessorImpl#lambda$service$0:130
java.util.stream.ReduceOps$1ReducingSink#accept:80
java.util.Spliterators$ArraySpliterator#forEachRemaining:948
java.util.stream.AbstractPipeline#copyInto:484
java.util.stream.AbstractPipeline#wrapAndCopyInto:474
java.util.stream.ReduceOps$ReduceOp#evaluateSequential:913
java.util.stream.AbstractPipeline#evaluate:234
java.util.stream.ReferencePipeline#reduce:563
org.apache.james.mailetcontainer.impl.MailetProcessorImpl#service:128
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on pull request #940: JAMES-3724 - Fixing leak resource for MemoryMailRepository
Posted by GitBox <gi...@apache.org>.
vttranlina commented on pull request #940:
URL: https://github.com/apache/james-project/pull/940#issuecomment-1085518526
Many classes have been detected resource leaked.
Almost come from unit-test, when it created new MailImpl for testing.
Any other way is better than override detect mode everywhere?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on pull request #940: JAMES-3724 - Fixing leak resource for MemoryMailRepository
Posted by GitBox <gi...@apache.org>.
vttranlina commented on pull request #940:
URL: https://github.com/apache/james-project/pull/940#issuecomment-1080359823
Regarding the Resource leak detected at `MemoryMailRepositoryTest`,
I tried to manually dispose of resources, but then the source code of the unit test look very noisy.
IMO, disabling leak detection at this test is better. It is just a unit test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #940: JAMES-3724 - Fixing leak resource for MemoryMailRepository
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #940:
URL: https://github.com/apache/james-project/pull/940#issuecomment-1085521021
> Any other way is better than override detect mode everywhere?
Patch the tests to release their resources?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #940: JAMES-3724 - Fixing leak resource for MemoryMailRepository
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #940:
URL: https://github.com/apache/james-project/pull/940#discussion_r836188546
##########
File path: server/container/lifecycle-api/src/test/java/org/apache/james/lifecycle/api/LeakAwareFixture.java
##########
@@ -0,0 +1,36 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.lifecycle.api;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+
+public class LeakAwareFixture {
+
+ // using reflect to change LeakAware.LEVEL value
+ public static void forceChangeLevel(Disposable.LeakAware.Level level) throws NoSuchFieldException, IllegalAccessException {
+ final Field field = Disposable.LeakAware.class.getDeclaredField("LEVEL");
+ field.setAccessible(true);
+ final Field modifiersField = Field.class.getDeclaredField("modifiers");
+ modifiersField.setAccessible(true);
+ modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL);
+ field.set(null, level);
+ }
Review comment:
I prefer us to pass the system property to the surefire plugin rather than having reflection in the middle of production code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #940: JAMES-3724 - Fixing leak resource for MemoryMailRepository
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #940:
URL: https://github.com/apache/james-project/pull/940#discussion_r836057550
##########
File path: server/mailrepository/mailrepository-memory/src/main/java/org/apache/james/mailrepository/memory/MemoryMailRepository.java
##########
@@ -26,16 +26,18 @@
import javax.mail.MessagingException;
+import org.apache.james.lifecycle.api.LifecycleUtil;
import org.apache.james.mailrepository.api.MailKey;
import org.apache.james.mailrepository.api.MailRepository;
import org.apache.mailet.Mail;
public class MemoryMailRepository implements MailRepository {
- private final ConcurrentHashMap<MailKey, Mail> mails;
+ private static final ConcurrentHashMap<MailKey, Mail> mails = new ConcurrentHashMap<>();
Review comment:
No all mail repository should not share a common memory stucture
And this static field cheats by preventing GC to find those.
What we should likely do is:
- Always dispose emails upon delete, clear
- Call clear in our @AfterEach callbacks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on a change in pull request #940: JAMES-3724 - Fixing leak resource for MemoryMailRepository
Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #940:
URL: https://github.com/apache/james-project/pull/940#discussion_r836189934
##########
File path: server/container/lifecycle-api/src/test/java/org/apache/james/lifecycle/api/LeakAwareFixture.java
##########
@@ -0,0 +1,36 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.lifecycle.api;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+
+public class LeakAwareFixture {
+
+ // using reflect to change LeakAware.LEVEL value
+ public static void forceChangeLevel(Disposable.LeakAware.Level level) throws NoSuchFieldException, IllegalAccessException {
+ final Field field = Disposable.LeakAware.class.getDeclaredField("LEVEL");
+ field.setAccessible(true);
+ final Field modifiersField = Field.class.getDeclaredField("modifiers");
+ modifiersField.setAccessible(true);
+ modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL);
+ field.set(null, level);
+ }
Review comment:
(y), let me override it in pom.xml
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org