Skip to content

Commit

Permalink
#135: workaround memory leak issue in groovy 2.0.7
Browse files Browse the repository at this point in the history
  • Loading branch information
ypujante committed Mar 27, 2013
1 parent fbeae8c commit f437027
Show file tree
Hide file tree
Showing 13 changed files with 99 additions and 88 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* Copyright (c) 2010-2010 LinkedIn, Inc
* Portions Copyright (c) 2011 Yan Pujante
* Portions Copyright (c) 2011-2013 Yan Pujante
*
* Licensed 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
Expand All @@ -20,6 +20,7 @@ package org.linkedin.glu.agent.impl.capabilities

import eu.medsea.mimeutil.MimeUtil
import org.linkedin.glu.groovy.utils.collections.GluGroovyCollectionUtils
import org.linkedin.groovy.util.ant.AntUtils

import java.util.concurrent.TimeoutException
import java.util.regex.Pattern
Expand Down Expand Up @@ -319,7 +320,7 @@ def class ShellImpl implements Shell
*/
def ant(Closure closure)
{
return AntHelper.withBuilder { closure(it) }
return AntUtils.withBuilder { closure(it) }
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012 Yan Pujante
* Copyright (c) 2012-2013 Yan Pujante
*
* Licensed 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
Expand Down Expand Up @@ -34,6 +34,12 @@ public class CommandGluScriptFactory implements ScriptFactory
new CommandGluScript(ioStorage: ioStorage)
}

@Override
void destroyScript(ScriptConfig scriptConfig)
{
// nothing to do here
}

@Override
def toExternalRepresentation()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
* Copyright (c) 2010-2010 LinkedIn, Inc
* Portions Copyright (c) 2013 Yan Pujante
*
* Licensed 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
Expand Down Expand Up @@ -72,6 +73,13 @@ def class FromClassNameScriptFactory implements ScriptFactory, Serializable
return _script
}

@Override
void destroyScript(ScriptConfig scriptConfig)
{
_jarFiles?.each { scriptConfig.shell.rm(it) }
_classLoader = null
}

String toString()
{
if(_classPath)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* Copyright (c) 2010-2010 LinkedIn, Inc
* Portions Copyright (c) 2011 Yan Pujante
* Portions Copyright (c) 2011-2013 Yan Pujante
*
* Licensed 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
Expand Down Expand Up @@ -31,6 +31,7 @@ def class FromLocationScriptFactory implements ScriptFactory, Serializable
private final def _location
private def _scriptFile
private transient def _script
private transient GroovyClassLoader _gcl

FromLocationScriptFactory(location)
{
Expand All @@ -44,13 +45,28 @@ def class FromLocationScriptFactory implements ScriptFactory, Serializable
if(!_scriptFile?.exists())
_scriptFile = scriptConfig.shell.fetch(_location)

Class scriptClass = new GroovyClassLoader(getClass().classLoader).parseClass(_scriptFile.file)
_gcl = new GroovyClassLoader(getClass().classLoader)

Class scriptClass = _gcl.parseClass(_scriptFile.file)

_script = scriptClass.newInstance()
}

return _script
}

@Override
void destroyScript(ScriptConfig scriptConfig)
{
if(_scriptFile?.exists())
scriptConfig.shell.rm(_scriptFile)

_scriptFile = null
_script = null
_gcl.clearCache()
_gcl = null
}

String toString()
{
return "FromLocationScriptFactory[${_location}]".toString();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
* Copyright (c) 2010-2010 LinkedIn, Inc
* Portions Copyright (c) 2013 Yan Pujante
*
* Licensed 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
Expand Down Expand Up @@ -31,52 +32,51 @@ class ScriptDefinition implements Serializable, Externable

final MountPoint mountPoint
final MountPoint parent
final ScriptFactory scriptFactory
final ScriptFactory scriptFactory = null // set to null always so that it does not get serialized
final def initParameters
final def _scriptFactoryArgs
final transient ScriptFactory _scriptFactory // not serialized

ScriptDefinition(MountPoint mountPoint,
MountPoint parent,
ScriptFactory scriptFactory,
initParameters)
{
// parent is required unless mountpoint is root...
// parent is required unless mountPoint is root...
assert parent != null || mountPoint == MountPoint.ROOT

this.mountPoint = mountPoint
this.parent = parent
this.scriptFactory = null
this._scriptFactory = scriptFactory
this.initParameters = initParameters ?: [:]
this._scriptFactoryArgs = scriptFactory.toExternalRepresentation()
}

def getScriptFactoryArgs()
{
if(_scriptFactoryArgs == null)
return scriptFactory.toExternalRepresentation()
else
_scriptFactoryArgs
_scriptFactoryArgs
}

ScriptFactory getScriptFactory(ScriptFactoryFactory scriptFactoryFactory)
ScriptFactory getScriptFactory()
{
// for backward compatibility
if(_scriptFactoryArgs == null)
return scriptFactory
else
return scriptFactoryFactory.createScriptFactory(_scriptFactoryArgs)
return _scriptFactory
}

void setScriptFactory(ScriptFactory factory)
{
throw new UnsupportedOperationException("read only")
}

public String toString()
{
return "[mountPoint: ${mountPoint}, parent: ${parent}, scriptFactory: ${scriptFactory}, scriptFactoryArgs: ${_scriptFactoryArgs}, initParameters: ${initParameters}]";
return "[mountPoint: ${mountPoint}, parent: ${parent}, scriptFactoryArgs: ${scriptFactoryArgs}, initParameters: ${initParameters}]";
}

def toExternalRepresentation()
{
return [mountPoint: mountPoint,
parent: parent,
scriptFactory: scriptFactory?.toExternalRepresentation() ?: _scriptFactoryArgs,
scriptFactory: scriptFactoryArgs,
initParameters: initParameters]
}

Expand All @@ -90,7 +90,6 @@ class ScriptDefinition implements Serializable, Externable

if(that.mountPoint != this.mountPoint) return false;
if(that.parent != this.parent) return false;
if(that.scriptFactory != this.scriptFactory) return false;
if(that._scriptFactoryArgs != this._scriptFactoryArgs) return false;
if(that.initParameters != this.initParameters) return false;

Expand All @@ -103,8 +102,7 @@ class ScriptDefinition implements Serializable, Externable

result = (mountPoint ? mountPoint.hashCode() : 0);
result = 31 * result + (parent ? parent.hashCode() : 0);
result = 31 * result + (scriptFactory ? scriptFactory.hashCode() : 0);
result = 31 * result + (scriptFactoryArgs ? scriptFactoryArgs.hashCode() : 0);
result = 31 * result + (_scriptFactoryArgs ? _scriptFactoryArgs.hashCode() : 0);
result = 31 * result + (initParameters ? initParameters.hashCode() : 0);
return result;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
* Copyright (c) 2010-2010 LinkedIn, Inc
* Portions Copyright (c) 2013 Yan Pujante
*
* Licensed 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
Expand Down Expand Up @@ -28,6 +29,11 @@ interface ScriptFactory
*/
def createScript(ScriptConfig scriptConfig)

/**
* Called to destroy the script
*/
void destroyScript(ScriptConfig scriptConfig)

/**
* @returns an external representation of the factory
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* Copyright (c) 2010-2010 LinkedIn, Inc
* Portions Copyright (c) 2011 Yan Pujante
* Portions Copyright (c) 2011-2013 Yan Pujante
*
* Licensed 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
Expand Down Expand Up @@ -147,7 +147,7 @@ def class ScriptManagerImpl implements ScriptManager

try
{
childScript = sd.getScriptFactory(scriptFactoryFactory).createScript(scriptConfig)
childScript = sd.scriptFactory.createScript(scriptConfig)
}
catch(Exception e)
{
Expand Down Expand Up @@ -325,6 +325,12 @@ def class ScriptManagerImpl implements ScriptManager
// we clean up the temp space
node.shell.rmdirs(node.shell.fileSystem.tmpRoot)

def scriptConfig = new ScriptConfig(shell: node.shell,
agentContext: agentContext)

// give a chance to the factory to "release" some resources
node.scriptDefinition.scriptFactory.destroyScript(scriptConfig)

removeScriptNode(mountPoint)

node.log.info("uninstalled")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* Copyright (c) 2010-2010 LinkedIn, Inc
* Portions Copyright (c) 2011 Yan Pujante
* Portions Copyright (c) 2011-2013 Yan Pujante
*
* Licensed 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
Expand Down Expand Up @@ -85,6 +85,11 @@ class ZooKeeperStorage implements WriteOnlyStorage
log.warn("Call ignored ${closure.toString()} due to unexpected exception", e)
}

log.warn("Call ignored ${closure.toString()}: zookeeper is not connected")

if(log.isDebugEnabled())
log.debug("zkSafe => Call ignored due to zk not connected!", new Exception("where am I?"))

return null
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
* Copyright (c) 2010-2010 LinkedIn, Inc
* Portions Copyright (c) 2013 Yan Pujante
*
* Licensed 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
Expand All @@ -18,7 +19,7 @@ package test.agent.perf

/**
* Example of running the performance test:
* groovy groovy/test/agent/perf/TestPerfTimers.groovy /Volumes/raptor/deployment/glu/dist/agent/client/bin/gluc.sh resources/TimersTestScript.groovy
* groovy groovy/test/agent/perf/TestPerfTimers.groovy /export/content/glu/org.linkedin.glu.packaging-all-xxx/bin/agent-cli.sh resources/TimersTestScript.groovy
* @author ypujante@linkedin.com */
class TestPerfTimers
{
Expand Down
9 changes: 8 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ buildscript {
}

dependencies {
classpath 'org.linkedin:org.linkedin.gradle-plugins:1.5.glu47.3'
classpath 'org.linkedin:org.linkedin.gradle-plugins:1.6.0'
}
}

Expand Down Expand Up @@ -75,6 +75,13 @@ allprojects {
apply plugin: 'idea'
group = spec.group
version = spec.version

// this is to force JUnit to a consistent version (zookeeper is bringing a different one!)
configurations.all {
resolutionStrategy {
force spec.external.junit
}
}
}

def ideaCopyright = """
Expand Down
4 changes: 2 additions & 2 deletions project-spec.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ spec = [
grails: '2.2.1',
groovy: '2.0.7',
jetty: '8.1.10.v20130312', // '9.0.0.v20130308' (cannot use 9 -> requires jdk 1.7)
linkedinUtils: '1.8.glu47.2',
linkedinZookeeper: '1.5.glu47.1',
linkedinUtils: '1.8.glu47.3',
linkedinZookeeper: '1.5.glu47.2',
restlet: '2.1.2',
sigar: '1.6.4',
slf4j: '1.6.2' // to be compatible with grails 2.2.1
Expand Down
Loading

0 comments on commit f437027

Please sign in to comment.