killbill-memoizeit

diff --git a/osgi-bundles/bundles/jruby/src/main/java/com/ning/billing/osgi/bundles/jruby/JRubyActivator.java b/osgi-bundles/bundles/jruby/src/main/java/com/ning/billing/osgi/bundles/jruby/JRubyActivator.java
index 3fc78f5..33341aa 100644
--- a/osgi-bundles/bundles/jruby/src/main/java/com/ning/billing/osgi/bundles/jruby/JRubyActivator.java
+++ b/osgi-bundles/bundles/jruby/src/main/java/com/ning/billing/osgi/bundles/jruby/JRubyActivator.java
@@ -17,14 +17,13 @@
 package com.ning.billing.osgi.bundles.jruby;
 
 import java.io.File;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 
-import org.jruby.embed.ScriptingContainer;
 import org.osgi.framework.BundleContext;
 import org.osgi.service.log.LogService;
 
@@ -63,13 +62,12 @@ public class JRubyActivator extends KillbillActivatorBase {
 
                 // Setup JRuby
                 final String pluginMain;
-                final ScriptingContainer scriptingContainer = setupScriptingContainer(rubyConfig);
                 if (PluginType.NOTIFICATION.equals(rubyConfig.getPluginType())) {
-                    plugin = new JRubyNotificationPlugin(rubyConfig, scriptingContainer, context, logService);
+                    plugin = new JRubyNotificationPlugin(rubyConfig, context, logService);
                     dispatcher.registerEventHandler((OSGIKillbillEventHandler) plugin);
                     pluginMain = KILLBILL_PLUGIN_JNOTIFICATION;
                 } else if (PluginType.PAYMENT.equals(rubyConfig.getPluginType())) {
-                    plugin = new JRubyPaymentPlugin(rubyConfig, scriptingContainer, context, logService);
+                    plugin = new JRubyPaymentPlugin(rubyConfig, context, logService);
                     pluginMain = KILLBILL_PLUGIN_JPAYMENT;
                 } else {
                     throw new IllegalStateException("Unsupported plugin type " + rubyConfig.getPluginType());
@@ -88,9 +86,6 @@ public class JRubyActivator extends KillbillActivatorBase {
         // Default to the plugin root dir if no jruby plugins specific configuration directory was specified
         killbillServices.put("conf_dir", Objects.firstNonNull(JRUBY_PLUGINS_CONF_DIR, rubyConfig.getPluginVersionRoot().getAbsolutePath()));
 
-        // Initial start
-        doStartPlugin(pluginMain, context, killbillServices);
-
         // Setup the restart mechanism. This is useful for hotswapping plugin code
         // The principle is similar to the one in Phusion Passenger:
         // http://www.modrails.com/documentation/Users%20guide%20Apache.html#_redeploying_restarting_the_ruby_on_rails_application
@@ -106,19 +101,28 @@ public class JRubyActivator extends KillbillActivatorBase {
             return;
         }
 
+        final AtomicBoolean firstStart = new AtomicBoolean(true);
         // TODO Switch to failsafe once in killbill-commons
         restartFuture = Executors.newSingleThreadScheduledExecutor().scheduleWithFixedDelay(new Runnable() {
             long lastRestartMillis = System.currentTimeMillis();
 
             @Override
             public void run() {
+                if (firstStart.get()) {
+                    // Initial start
+                    logService.log(LogService.LOG_INFO, "Starting JRuby plugin " + rubyConfig.getRubyMainClass());
+                    doStartPlugin(pluginMain, context, killbillServices);
+                    firstStart.set(false);
+                    return;
+                }
+
                 final File restartFile = new File(tmpDirPath + "/" + RESTART_FILE_NAME);
                 if (!restartFile.isFile()) {
                     return;
                 }
 
                 if (restartFile.lastModified() > lastRestartMillis) {
-                    logService.log(LogService.LOG_INFO, "Restarting JRuby plugin " + plugin.getPluginMainClass());
+                    logService.log(LogService.LOG_INFO, "Restarting JRuby plugin " + rubyConfig.getRubyMainClass());
 
                     doStopPlugin(context);
                     doStartPlugin(pluginMain, context, killbillServices);
@@ -126,7 +130,7 @@ public class JRubyActivator extends KillbillActivatorBase {
                     lastRestartMillis = restartFile.lastModified();
                 }
             }
-        }, JRUBY_PLUGINS_RESTART_DELAY_SECS, JRUBY_PLUGINS_RESTART_DELAY_SECS, TimeUnit.SECONDS);
+        }, 0, JRUBY_PLUGINS_RESTART_DELAY_SECS, TimeUnit.SECONDS);
     }
 
     private PluginRubyConfig retrievePluginRubyConfig(final BundleContext context) {
@@ -134,15 +138,6 @@ public class JRubyActivator extends KillbillActivatorBase {
         return pluginConfigServiceApi.getPluginRubyConfig(context.getBundle().getBundleId());
     }
 
-    private ScriptingContainer setupScriptingContainer(final PluginRubyConfig rubyConfig) {
-        final ScriptingContainer scriptingContainer = new ScriptingContainer();
-
-        // Set the load paths instead of adding, to avoid looking at the filesystem
-        scriptingContainer.setLoadPaths(Collections.<String>singletonList(rubyConfig.getRubyLoadDir()));
-
-        return scriptingContainer;
-    }
-
     public void stop(final BundleContext context) throws Exception {
 
         withContextClassLoader(new PluginCall() {
@@ -157,13 +152,13 @@ public class JRubyActivator extends KillbillActivatorBase {
     }
 
     private void doStartPlugin(final String pluginMain, final BundleContext context, final Map<String, Object> killbillServices) {
-        logService.log(LogService.LOG_INFO, "Starting JRuby plugin " + plugin.getPluginMainClass());
         plugin.instantiatePlugin(killbillServices, pluginMain);
         plugin.startPlugin(context);
     }
 
     private void doStopPlugin(final BundleContext context) {
         plugin.stopPlugin(context);
+        plugin.unInstantiatePlugin();
     }
 
     // We make the explicit registration in the start method by hand as this would be called too early

diff --git a/osgi-bundles/bundles/jruby/src/main/java/com/ning/billing/osgi/bundles/jruby/JRubyNotificationPlugin.java b/osgi-bundles/bundles/jruby/src/main/java/com/ning/billing/osgi/bundles/jruby/JRubyNotificationPlugin.java
index fa6118f..d5f9d80 100644
--- a/osgi-bundles/bundles/jruby/src/main/java/com/ning/billing/osgi/bundles/jruby/JRubyNotificationPlugin.java
+++ b/osgi-bundles/bundles/jruby/src/main/java/com/ning/billing/osgi/bundles/jruby/JRubyNotificationPlugin.java
@@ -17,33 +17,23 @@
 package com.ning.billing.osgi.bundles.jruby;
 
 import org.jruby.Ruby;
-import org.jruby.embed.ScriptingContainer;
-import org.jruby.javasupport.JavaEmbedUtils;
 import org.osgi.framework.BundleContext;
 import org.osgi.service.log.LogService;
 
 import com.ning.billing.beatrix.bus.api.ExtBusEvent;
 import com.ning.billing.notification.plugin.api.NotificationPluginApi;
 import com.ning.billing.osgi.api.config.PluginRubyConfig;
-import com.ning.billing.payment.plugin.api.PaymentPluginApi;
 import com.ning.billing.payment.plugin.api.PaymentPluginApiException;
 import com.ning.killbill.osgi.libs.killbill.OSGIKillbillEventDispatcher.OSGIKillbillEventHandler;
 
 public class JRubyNotificationPlugin extends JRubyPlugin implements OSGIKillbillEventHandler {
 
-    public JRubyNotificationPlugin(final PluginRubyConfig config, final ScriptingContainer container,
-                                   final BundleContext bundleContext, final LogService logger) {
-        super(config, container, bundleContext, logger);
-    }
-
-    @Override
-    public void startPlugin(final BundleContext context) {
-        super.startPlugin(context);
+    public JRubyNotificationPlugin(final PluginRubyConfig config, final BundleContext bundleContext, final LogService logger) {
+        super(config, bundleContext, logger);
     }
 
     @Override
     public void handleKillbillEvent(final ExtBusEvent killbillEvent) {
-
         try {
             callWithRuntimeAndChecking(new PluginCallback(VALIDATION_PLUGIN_TYPE.NOTIFICATION) {
                 @Override

diff --git a/osgi-bundles/bundles/jruby/src/main/java/com/ning/billing/osgi/bundles/jruby/JRubyPaymentPlugin.java b/osgi-bundles/bundles/jruby/src/main/java/com/ning/billing/osgi/bundles/jruby/JRubyPaymentPlugin.java
index 3b52ee3..ae9fecf 100644
--- a/osgi-bundles/bundles/jruby/src/main/java/com/ning/billing/osgi/bundles/jruby/JRubyPaymentPlugin.java
+++ b/osgi-bundles/bundles/jruby/src/main/java/com/ning/billing/osgi/bundles/jruby/JRubyPaymentPlugin.java
@@ -23,7 +23,6 @@ import java.util.List;
 import java.util.UUID;
 
 import org.jruby.Ruby;
-import org.jruby.embed.ScriptingContainer;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.ServiceRegistration;
 import org.osgi.service.log.LogService;
@@ -44,9 +43,8 @@ public class JRubyPaymentPlugin extends JRubyPlugin implements PaymentPluginApi 
 
     private volatile ServiceRegistration<PaymentPluginApi> paymentInfoPluginRegistration;
 
-    public JRubyPaymentPlugin(final PluginRubyConfig config, final ScriptingContainer container,
-                              final BundleContext bundleContext, final LogService logger) {
-        super(config, container, bundleContext, logger);
+    public JRubyPaymentPlugin(final PluginRubyConfig config, final BundleContext bundleContext, final LogService logger) {
+        super(config, bundleContext, logger);
     }
 
     @Override
@@ -62,11 +60,12 @@ public class JRubyPaymentPlugin extends JRubyPlugin implements PaymentPluginApi 
 
     @Override
     public void stopPlugin(final BundleContext context) {
-        paymentInfoPluginRegistration.unregister();
+        if (paymentInfoPluginRegistration != null) {
+            paymentInfoPluginRegistration.unregister();
+        }
         super.stopPlugin(context);
     }
 
-
     @Override
     public PaymentInfoPlugin processPayment(final UUID kbAccountId, final UUID kbPaymentId, final UUID kbPaymentMethodId, final BigDecimal amount, final Currency currency, final CallContext context) throws PaymentPluginApiException {
 

diff --git a/osgi-bundles/bundles/jruby/src/main/java/com/ning/billing/osgi/bundles/jruby/JRubyPlugin.java b/osgi-bundles/bundles/jruby/src/main/java/com/ning/billing/osgi/bundles/jruby/JRubyPlugin.java
index 8993df1..d3685eb 100644
--- a/osgi-bundles/bundles/jruby/src/main/java/com/ning/billing/osgi/bundles/jruby/JRubyPlugin.java
+++ b/osgi-bundles/bundles/jruby/src/main/java/com/ning/billing/osgi/bundles/jruby/JRubyPlugin.java
@@ -16,7 +16,7 @@
 
 package com.ning.billing.osgi.bundles.jruby;
 
-import java.util.Arrays;
+import java.util.Collections;
 import java.util.Hashtable;
 import java.util.Map;
 
@@ -25,6 +25,8 @@ import javax.servlet.http.HttpServlet;
 import org.jruby.Ruby;
 import org.jruby.RubyObject;
 import org.jruby.embed.EvalFailedException;
+import org.jruby.embed.LocalContextScope;
+import org.jruby.embed.LocalVariableBehavior;
 import org.jruby.embed.ScriptingContainer;
 import org.jruby.runtime.builtin.IRubyObject;
 import org.osgi.framework.BundleContext;
@@ -50,44 +52,38 @@ public abstract class JRubyPlugin {
     private static final String KILLBILL_SERVICES = "java_apis";
     private static final String KILLBILL_PLUGIN_CLASS_NAME = "plugin_class_name";
 
+    // Methods implemented by Killbill::Plugin::JPlugin
+    private static final String START_PLUGIN_RUBY_METHOD_NAME = "start_plugin";
+    private static final String STOP_PLUGIN_RUBY_METHOD_NAME = "stop_plugin";
+    private static final String RACK_HANDLER_RUBY_METHOD_NAME = "rack_handler";
+
+    private final Object pluginMonitor = new Object();
+
     protected final LogService logger;
     protected final BundleContext bundleContext;
     protected final String pluginGemName;
     protected final String rubyRequire;
     protected final String pluginMainClass;
-    protected final ScriptingContainer container;
     protected final String pluginLibdir;
 
+    protected ScriptingContainer container;
     protected RubyObject pluginInstance;
 
     private ServiceRegistration httpServletServiceRegistration = null;
     private String cachedRequireLine = null;
 
-    public JRubyPlugin(final PluginRubyConfig config, final ScriptingContainer container,
-                       final BundleContext bundleContext, final LogService logger) {
+    public JRubyPlugin(final PluginRubyConfig config, final BundleContext bundleContext, final LogService logger) {
         this.logger = logger;
         this.bundleContext = bundleContext;
         this.pluginGemName = config.getPluginName();
         this.rubyRequire = config.getRubyRequire();
         this.pluginMainClass = config.getRubyMainClass();
-        this.container = container;
         this.pluginLibdir = config.getRubyLoadDir();
-
-        // Path to the gem
-        if (pluginLibdir != null) {
-            container.setLoadPaths(Arrays.asList(pluginLibdir));
-        }
-    }
-
-    public String getPluginMainClass() {
-        return pluginMainClass;
-    }
-
-    public String getPluginLibdir() {
-        return pluginLibdir;
     }
 
     public void instantiatePlugin(final Map<String, Object> killbillApis, final String pluginMain) {
+        container = setupScriptingContainer();
+
         checkValidPlugin();
 
         // Register all killbill APIs
@@ -103,7 +99,7 @@ public abstract class JRubyPlugin {
 
     public void startPlugin(final BundleContext context) {
         checkPluginIsStopped();
-        pluginInstance.callMethod("start_plugin");
+        pluginInstance.callMethod(START_PLUGIN_RUBY_METHOD_NAME);
         checkPluginIsRunning();
         registerHttpServlet();
     }
@@ -111,13 +107,18 @@ public abstract class JRubyPlugin {
     public void stopPlugin(final BundleContext context) {
         checkPluginIsRunning();
         unregisterHttpServlet();
-        pluginInstance.callMethod("stop_plugin");
+        pluginInstance.callMethod(STOP_PLUGIN_RUBY_METHOD_NAME);
         checkPluginIsStopped();
     }
 
+    public void unInstantiatePlugin() {
+        // Cleanup the container
+        container.terminate();
+    }
+
     private void registerHttpServlet() {
         // Register the rack handler
-        final IRubyObject rackHandler = pluginInstance.callMethod("rack_handler");
+        final IRubyObject rackHandler = pluginInstance.callMethod(RACK_HANDLER_RUBY_METHOD_NAME);
         if (!rackHandler.isNil()) {
             logger.log(LogService.LOG_INFO, String.format("Using %s as rack handler", rackHandler.getMetaClass()));
 
@@ -134,19 +135,19 @@ public abstract class JRubyPlugin {
         }
     }
 
-    protected void checkPluginIsRunning() {
+    private void checkPluginIsRunning() {
         if (pluginInstance == null || !(Boolean) pluginInstance.callMethod("is_active").toJava(Boolean.class)) {
             throw new IllegalStateException(String.format("Plugin %s didn't start properly", pluginMainClass));
         }
     }
 
-    protected void checkPluginIsStopped() {
+    private void checkPluginIsStopped() {
         if (pluginInstance == null || (Boolean) pluginInstance.callMethod("is_active").toJava(Boolean.class)) {
             throw new IllegalStateException(String.format("Plugin %s didn't stop properly", pluginMainClass));
         }
     }
 
-    protected void checkValidPlugin() {
+    private void checkValidPlugin() {
         try {
             container.runScriptlet(checkInstanceOfPlugin(KILLBILL_PLUGIN_BASE));
         } catch (EvalFailedException e) {
@@ -154,7 +155,7 @@ public abstract class JRubyPlugin {
         }
     }
 
-    protected void checkValidNotificationPlugin() throws IllegalArgumentException {
+    private void checkValidNotificationPlugin() throws IllegalArgumentException {
         try {
             container.runScriptlet(checkInstanceOfPlugin(KILLBILL_PLUGIN_NOTIFICATION));
         } catch (EvalFailedException e) {
@@ -162,7 +163,7 @@ public abstract class JRubyPlugin {
         }
     }
 
-    protected void checkValidPaymentPlugin() throws IllegalArgumentException {
+    private void checkValidPaymentPlugin() throws IllegalArgumentException {
         try {
             container.runScriptlet(checkInstanceOfPlugin(KILLBILL_PLUGIN_PAYMENT));
         } catch (EvalFailedException e) {
@@ -170,7 +171,7 @@ public abstract class JRubyPlugin {
         }
     }
 
-    protected String checkInstanceOfPlugin(final String baseClass) {
+    private String checkInstanceOfPlugin(final String baseClass) {
         final StringBuilder builder = new StringBuilder(getRequireLine());
         builder.append("raise ArgumentError.new('Invalid plugin: ")
                .append(pluginMainClass)
@@ -220,10 +221,21 @@ public abstract class JRubyPlugin {
         return cachedRequireLine;
     }
 
-    protected Ruby getRuntime() {
+    private Ruby getRuntime() {
         return pluginInstance.getMetaClass().getRuntime();
     }
 
+    private ScriptingContainer setupScriptingContainer() {
+        // SINGLETHREAD model to avoid sharing state across scripting containers
+        // All calls are synchronized anyways (don't trust gems to be thread safe)
+        final ScriptingContainer scriptingContainer = new ScriptingContainer(LocalContextScope.SINGLETHREAD, LocalVariableBehavior.TRANSIENT, true);
+
+        // Set the load paths instead of adding, to avoid looking at the filesystem
+        scriptingContainer.setLoadPaths(Collections.<String>singletonList(pluginLibdir));
+
+        return scriptingContainer;
+    }
+
     public enum VALIDATION_PLUGIN_TYPE {
         NOTIFICATION,
         PAYMENT,
@@ -246,25 +258,27 @@ public abstract class JRubyPlugin {
     }
 
     protected <T> T callWithRuntimeAndChecking(final PluginCallback cb) throws PaymentPluginApiException {
-        try {
-            checkPluginIsRunning();
-
-            switch(cb.getPluginType()) {
-                case NOTIFICATION:
-                    checkValidNotificationPlugin();
-                    break;
-                case PAYMENT:
-                    checkValidPaymentPlugin();
-                    break;
-                default:
-                    break;
+        synchronized (pluginMonitor) {
+            try {
+                checkPluginIsRunning();
+
+                switch (cb.getPluginType()) {
+                    case NOTIFICATION:
+                        checkValidNotificationPlugin();
+                        break;
+                    case PAYMENT:
+                        checkValidPaymentPlugin();
+                        break;
+                    default:
+                        break;
+                }
+
+                final Ruby runtime = getRuntime();
+                return cb.doCall(runtime);
+            } catch (RuntimeException e) {
+                log.warn("RuntimeException in jruby plugin ", e);
+                throw e;
             }
-
-            final Ruby runtime = getRuntime();
-            return cb.doCall(runtime);
-        } catch (RuntimeException e) {
-            log.warn("RuntimeException in jruby plugin ", e);
-            throw e;
         }
     }
 }