opendev/gerrit
Code Issues Proposed changes

Make VelocityRuntimeProvider singleton, not RuntimeInstance

Browse Source 
If sendmail.threadPoolSize is set to a value > 1 sending emails may
sometimes fail with an error like this:

  Cannot email comments for xxxxxx,x
  com.google.gerrit.common.errors.EmailException: Mail Error: Cannot format velocity template com/google/gerrit/server/mail/ChangeSubject.vm
    at com.google.gerrit.server.mail.OutgoingEmail.velocifyFile(OutgoingEmail.java:452)
    at com.google.gerrit.server.mail.ChangeEmail.setChangeSubjectHeader(ChangeEmail.java:185)
    at com.google.gerrit.server.mail.ChangeEmail.init(ChangeEmail.java:163)
    at com.google.gerrit.server.mail.ReplyToChangeSender.init(ReplyToChangeSender.java:35)
    at com.google.gerrit.server.mail.CommentSender.init(CommentSender.java:88)
    at com.google.gerrit.server.mail.OutgoingEmail.send(OutgoingEmail.java:93)
    at com.google.gerrit.server.change.EmailReviewComments.run(EmailReviewComments.java:112)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    ...

This is because RuntimeInstance is marked as singleton and 2 threads
may concurrently change its state.

Bug: issue 4241
Change-Id: I9d735a081abfe59c4c162260500a8cb205295075
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin 2016-07-29 13:57:58 +02:00
parent 4b45e31435
commit 8cf2a56bda
2 changed files with 3 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
View File

@ -232,8 +232,7 @@ public class GerritGlobalModule extends FactoryModule {
bind(ApprovalsUtil.class);
bind(RuntimeInstance.class)
.toProvider(VelocityRuntimeProvider.class)
.in(SINGLETON);
.toProvider(VelocityRuntimeProvider.class);
bind(FromAddressGenerator.class).toProvider(
FromAddressGeneratorProvider.class).in(SINGLETON);
bind(Boolean.class).annotatedWith(DisableReverseDnsLookup.class)

2
gerrit-server/src/main/java/com/google/gerrit/server/mail/VelocityRuntimeProvider.java
View File

@ -18,6 +18,7 @@ import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.ProvisionException;
import com.google.inject.Singleton;
import org.apache.velocity.runtime.RuntimeConstants;
import org.apache.velocity.runtime.RuntimeInstance;
@ -30,6 +31,7 @@ import java.nio.file.Files;
import java.util.Properties;
/** Configures Velocity template engine for sending email. */
@Singleton
public class VelocityRuntimeProvider implements Provider<RuntimeInstance> {
private final SitePaths site;