Hi Bruno,

thanks a lot for the patch! I will integrate your patch slightly modified (cause I did some improvements in LockManagerInMemory too).

regards,
Armin


Bruno CROS wrote:
Hi Armin,

The attached file is a solution to the LockManagerInMemory memory leak...

Well, the leak was on keyLockMap when real timeout occurs...(without
abort/commit end)  The implemented solution is to iterate this keyLock map
and detect if locks list (the entries) are no more usefull. Entries that are
made of obsolete locks only, are evicted (detection by isEmpty() method).

Original parameters,  object "cleanup frequency" and "max lock to clean"
have been modified to treat more Write locks.

A new parameter is the  "keyCleanupFrequency" : frequency to run keyCleanup
over object (original) cleanup. Initial value is 6, it triggers a keyCleanup
process every 6 * 5 seconds (30s).

New variables :
- keyCleanupCounter : counter to reach frequency
- lastCleanupTime : the time that last keyCleanup process took.
- lastCleanupCount : the number of evicted useless lists at last cleanup.

Modification have been tested with millions of locks / thousands of keys.

May you post the code with a little comments cleanup ;-)

Regards.



On 3/9/07, Armin Waibel <[EMAIL PROTECTED]> wrote:

Hi Bruno,

Bruno CROS wrote:
> Hi all,
>
> Well, our team experiment some memory leaks using the LockManager in
remote
> mode, with the servlet and so, the LockManagerRemoteImpl.
>
> It seems that, after a while, in the LockManagerInMemoryImpl (use by
> LMRemoteImpl), some LockObject instances are stored in the static
HashMap
> keyLockMap  whereas the static LinkedMap resourceLockMap no more
contains
> references to that instances. What is sure, is that those locks never
> expire.
>

I will try to reproduce your problem - stay tuned.

regards,
Armin

> It results that memory usage grows and LockManager server crashes with
an
> OutOfMemoryException.
>
> Using the last implementation of LMRemoteImpl, LMInMemoryImpl, LMServlet
> over OJB 1.0.4. JVM 1.4.
>
> Thanks for any ideas.
>
> Regards.
>
> Bruno.
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]




------------------------------------------------------------------------

package org.apache.ojb.broker.locking;

/* Copyright 2002-2005 The Apache Software Foundation
 *
 * 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 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.
 */

import java.io.Serializable;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;

import org.apache.commons.collections.list.TreeList;
import org.apache.commons.collections.map.LinkedMap;
import org.apache.commons.lang.SystemUtils;
import org.apache.ojb.broker.util.configuration.Configurable;
import org.apache.ojb.broker.util.configuration.Configuration;
import org.apache.ojb.broker.util.configuration.ConfigurationException;
import org.apache.ojb.broker.util.logging.Logger;
import org.apache.ojb.broker.util.logging.LoggerFactory;

/**
 * This implementation of the [EMAIL PROTECTED] LockManager} interface supports 
a simple, fast, non-blocking
 * pessimistic locking for single JVM applications.
 *
 * @version $Id: LockManagerInMemoryImpl.java,v 1.3 2007/03/09 09:58:59 CROS_B 
Exp $
 */
public class LockManagerInMemoryImpl implements LockManager, Configurable, 
Serializable
{
    private Logger log = LoggerFactory.getLogger(LockManagerInMemoryImpl.class);
    /** The period to search for timed out locks. */
    // BC : liberer les ressources plus souvent
    private long cleanupFrequency = 5000; // 10000; // milliseconds.

    /** The number of lock entries to check for timeout */
    // BC : liberer plus de ressources
    private int maxLocksToClean = 2000;
/** * keyLockMap cleaning process frequency
     */
    private static int keyCleanupFrequency = 6;
/**
     * keyLockMap cleaning process counter
     */
    private static int keyCleanupCounter = 0;
/** * last key cleanup time in ms */ private static long lastKeyCleanupTime = 0; /**
     * last key cleanup removed lists
     */
private static int lastKeyCleanupCount = 0; /**
     * MBAIRD: a LinkedHashMap returns objects in the order you put them in,
     * while still maintaining an O(1) lookup like a normal hashmap. We can then
     * use this to get the oldest entries very quickly, makes cleanup a breeze.
     */
    private final Map resourceLockMap = new LinkedMap(70);
    private final Map keyLockMap = new HashMap();
    private final LockIsolationManager lockStrategyManager = new 
LockIsolationManager();
    private long m_lastCleanupAt = System.currentTimeMillis();
    private long lockTimeout;
    private long timeoutCounterRead;
    private long timeoutCounterWrite;

    public LockManagerInMemoryImpl()
    {
        this.lockTimeout = DEFAULT_LOCK_TIMEOUT;
    }

    public void configure(Configuration pConfig) throws ConfigurationException
    {
    }

    /** The period to search for timed out locks. */
    public long getCleanupFrequency()
    {
        return cleanupFrequency;
    }

    /** Set the period to search for timed out locks. */
    public void setCleanupFrequency(long cleanupFrequency)
    {
        this.cleanupFrequency = cleanupFrequency;
    }

    /**
     * The number of lock entries to check for timeout
     * on each lock cleanup run.
     *
     * @see #getCleanupFrequency()
     */
    public int getMaxLocksToClean()
    {
        return maxLocksToClean;
    }

    /**
     * Set the number of lock entries to check for timeout
     * on each lock cleanup run.
     *
     * @see #setCleanupFrequency(long)
     */
    public void setMaxLocksToClean(int maxLocksToClean)
    {
        this.maxLocksToClean = maxLocksToClean;
    }

    public long getLockTimeout()
    {
        return lockTimeout;
    }

    public void setLockTimeout(long timeout)
    {
        this.lockTimeout = timeout;
    }

    /**
     * NOOP
     *
     * @return Always '0'
     */
    public long getBlockTimeout()
    {
        return 0;
    }

    /** NOOP */
    public void setBlockTimeout(long timeout)
    {
        log.info("block-timeout setting is not supported");
    }

    public String getLockInfo()
    {
        // synchronize locks before return locking information
        checkTimedOutLocks();

        String eol = SystemUtils.LINE_SEPARATOR;
        StringBuffer msg = new StringBuffer("Class: " + 
LockManagerInMemoryImpl.class.getName() + eol);
        msg.append("lock timeout: ").append(getLockTimeout()).append(" 
[ms]").append(eol);
        msg.append("cleanup frequency: ").append(getCleanupFrequency()).append(" 
[ms]").append(eol);
        msg.append("max locks to clean: 
").append(getMaxLocksToClean()).append(eol);
        msg.append(eol);
        msg.append("lock map size: 
").append(resourceLockMap.size()).append(eol);
        msg.append("key lock map size: ").append(keyLockMap.size()).append(eol);
        msg.append("key cleanup counter : 
").append(keyCleanupCounter).append(eol);
        msg.append("last key cleanup count : 
").append(lastKeyCleanupCount).append(eol);
        msg.append("last key cleanup time : ").append(lastKeyCleanupTime).append(" 
[ms]").append(eol);
        msg.append("timed out write locks: 
").append(timeoutCounterWrite).append(eol);
        msg.append("timed out read locks: 
").append(timeoutCounterRead).append(eol);
        return msg.toString();
    }

    public boolean readLock(final Object key, final Object resourceId, final 
int isolationLevel)
    {
        if(log.isDebugEnabled()) log.debug("LM.readLock(tx-" + key + ", " + resourceId + 
")");
        checkTimedOutLocks();
        LockEntry reader = new LockEntry(resourceId,
                key,
                System.currentTimeMillis(),
                LockEntry.LOCK_READ);
        LockIsolation ls = lockStrategyManager.getStrategyFor(isolationLevel);

        boolean result;
        ObjectLocks objectLocks;
        /**
         * MBAIRD: We need to synchronize the get/put so we don't have two 
threads
         * competing to check if something is locked and double-locking it.
         */
        synchronized(resourceLockMap)
        {
            objectLocks = (ObjectLocks) resourceLockMap.get(resourceId);
            if(objectLocks == null)
            {
                // no write or read lock, go on
                objectLocks = new ObjectLocks(resourceId);
                resourceLockMap.put(resourceId, objectLocks);
            }
            result = objectLocks.addReader(reader, ls.allowMultipleRead(), 
ls.allowReadWhenWrite());
            if(result) associateKeyWithObjectLock(reader.getKey(), objectLocks);
        }
        return result;
    }

    public boolean releaseLock(final Object key, final Object resourceId)
    {
        if(log.isDebugEnabled()) log.debug("LM.releaseLock(tx-" + key + ", " + resourceId 
+ ")");

        synchronized(resourceLockMap)
        {
            ObjectLocks lock = (ObjectLocks) resourceLockMap.get(resourceId);
            return lock != null && lock.releaseLock(key);
        }
    }

    /** @see LockManager#releaseLocks(Object) */
    public void releaseLocks(final Object key)
    {
        if(log.isDebugEnabled()) log.debug("LM.releaseLocks(tx-" + key + ")");
        checkTimedOutLocks();
        doReleaseLocks(key);
        //System.out.println("resourceLockMap: " + resourceLockMap.size());
        //System.out.println("keyLockMap: " + keyLockMap.size());
    }

    public boolean writeLock(final Object key, final Object resourceId, final 
int isolationLevel)
    {
        if(log.isDebugEnabled()) log.debug("LM.writeLock(tx-" + key + ", " + resourceId + 
")");
        checkTimedOutLocks();
        LockEntry writer = new LockEntry(resourceId,
                key,
                System.currentTimeMillis(),
                LockEntry.LOCK_WRITE);
        LockIsolation ls = lockStrategyManager.getStrategyFor(isolationLevel);

        boolean result;
        ObjectLocks objectLocks;
        /**
         * MBAIRD: We need to synchronize the get/put so we don't have two 
threads
         * competing to check if something is locked and double-locking it.
         */
        synchronized(resourceLockMap)
        {
            objectLocks = (ObjectLocks) resourceLockMap.get(resourceId);
            // if we don't upgrade, go on
            if(objectLocks == null)
            {
                // no locks for current entity exist, so go on
                objectLocks = new ObjectLocks(resourceId);
                resourceLockMap.put(resourceId, objectLocks);
            }
            result = objectLocks.addWriter(writer, ls.allowWriteWhenRead());
            if(result) associateKeyWithObjectLock(writer.getKey(), objectLocks);
        }
        return result;
    }

    public boolean upgradeLock(final Object key, final Object resourceId, final 
int isolationLevel)
    {
        if(log.isDebugEnabled()) log.debug("LM.upgradeLock(tx-" + key + ", " + resourceId 
+ ")");
        return writeLock(key, resourceId, isolationLevel);
    }

    /** @see LockManager#hasWrite(Object, Object) */
    public boolean hasWrite(final Object key, final Object resourceId)
    {
        if(log.isDebugEnabled()) log.debug("LM.hasWrite(tx-" + key + ", " + resourceId + 
")");
        checkTimedOutLocks();

        ObjectLocks objectLocks;
        synchronized(resourceLockMap)
        {
            objectLocks = (ObjectLocks) resourceLockMap.get(resourceId);
            if(objectLocks != null)
            {
                return objectLocks.hasWriteLock(key);
            }
        }
        return false;
    }

    public boolean hasUpgrade(final Object key, final Object resourceId)
    {
        if(log.isDebugEnabled()) log.debug("LM.hasUpgrade(tx-" + key + ", " + resourceId 
+ ")");
        return hasWrite(key, resourceId);
    }

    /** @see LockManager#hasRead(Object, Object) */
    public boolean hasRead(final Object key, final Object resourceId)
    {
        if(log.isDebugEnabled()) log.debug("LM.hasRead(tx-" + key + ", " + 
resourceId + ')');
        checkTimedOutLocks();

        ObjectLocks objectLocks;
        synchronized(resourceLockMap)
        {
            objectLocks = (ObjectLocks) resourceLockMap.get(resourceId);
            if(objectLocks != null)
            {
                return objectLocks.hasReadLock(key);
            }
        }
        return false;
    }

    /**
     * The number of locked objects.
     */
    public int lockedObjects()
    {
        return resourceLockMap.size();
    }

    /**
     * Internal method to detect and remove timed out locks.
     */
    private void checkTimedOutLocks()
    {
        if(System.currentTimeMillis() - m_lastCleanupAt > cleanupFrequency)
        {
            removeTimedOutLocks(getLockTimeout());
            m_lastCleanupAt = System.currentTimeMillis();
        }
    }

    /**
     * removes all timed out lock entries from the persistent storage.
     */
    private void removeTimedOutLocks(long timeout)
    {
        int count = 0;
        long maxAge = System.currentTimeMillis() - timeout;
        ObjectLocks temp;
        synchronized(resourceLockMap)
        {

            int taille = resourceLockMap.size();
            boolean logVolume = taille > 10000;

            if (logVolume) {
              System.out.println("===== Avant RemoveTimedOutLocks =====");
              System.out.println("Nombre de locks   :" + taille );
              System.out.println("Mémoire JVM dispo :" + 
Runtime.getRuntime().freeMemory() );
              System.out.println("Mémoire utilisée  :" + 
Runtime.getRuntime().totalMemory());
              System.out.println("Mémoire totale    :" + 
Runtime.getRuntime().maxMemory());
            }
            Iterator it = resourceLockMap.values() .iterator();
            /**
             * run this loop while:
             * - we have more in the iterator
             * - the breakFromLoop flag hasn't been set
             * - we haven't removed more than the limit for this cleaning 
iteration.
             */
            while(it.hasNext() && (count <= maxLocksToClean))
            {
                temp = (ObjectLocks) it.next();
                if(temp.getWriter() != null)
                {
                    if(temp.getWriter().getTimestamp() < maxAge)
                    {
                        // writer has timed out, set it to null
                        temp.setWriter(null);
                        ++timeoutCounterWrite;
                    }
                }
                if(temp.readers != null && temp.readers.size() > 0)
                {
                    // all readers are older than timeout.
                    if(temp.lastReader < maxAge)
                    {
                        timeoutCounterRead += temp.getReaders().size();
                        if(temp.getWriter() == null)
                        {
                            // all readers and writer are older than timeout,
                            // remove the objectLock from the iterator (which
                            // is backed by the map, so it will be removed.
                            it.remove();
}
                        else
                        {
                            temp.getReaders().clear();
                        }
                    }
                    else
                    {
                        // there are outdated read locks
                        if(temp.eldestReader < maxAge)
                        {
                            // we need to walk each reader.
                            Iterator readerIt = 
temp.getReaders().values().iterator();
                            LockEntry readerLock;
                            while(readerIt.hasNext())
                            {
                                readerLock = (LockEntry) readerIt.next();
                                if(readerLock.getTimestamp() < maxAge)
                                {
                                    // this read lock is old, remove it.
                                    readerIt.remove();
                                    ++timeoutCounterRead;
                                }
                            }
                        }
                        else
                        {
                            // nothing to do
                        }
                    }
                }
                else
                {
                    if(temp.getWriter() == null)
                    {
                        it.remove();
                    }
                }
                count++;
            }
// keyCleanupCounter++;
                        if (keyCleanupCounter >= keyCleanupFrequency) {
                                keyCleanupCounter = 0;
                                lastKeyCleanupCount =0;
long startTime = System.currentTimeMillis(); // cleanup no more used keys (BC)
                                Iterator it2 = keyLockMap.values().iterator();
                                while (it2.hasNext()) {
                                        TreeList list = (TreeList) it2.next();
                                        boolean cleanList = true;
                                        // iterate ObjectLocks
                                        Iterator it3 = list.iterator();
                                        while (it3.hasNext()) {
                                                ObjectLocks ol = (ObjectLocks) 
it3.next();
                                                if (!ol.isEmpty()) {
                                                        cleanList = false;
                                                        break;
                                                }
                                        }
                                        // evict the no more used list
                                        if (cleanList) {
                                                it2.remove();
                                                lastKeyCleanupCount++;
                                        }
                                }
                                lastKeyCleanupTime = System.currentTimeMillis() 
- startTime;
                        }
if (logVolume) {
              System.out.println("===== Après RemoveTimedOutLocks =====");
              System.out.println("Nombre de locks   :" + 
resourceLockMap.size());
              System.out.println("Mémoire JVM dispo :" +
                                 Runtime.getRuntime().freeMemory());
              System.out.println("Mémoire utilisée  :" +
                                 Runtime.getRuntime().totalMemory());
              System.out.println("Mémoire totale    :" +
                                 Runtime.getRuntime().maxMemory());
            }

        }
    }

    private void associateKeyWithObjectLock(Object key, ObjectLocks lock)
    {
        List list = (List) keyLockMap.get(key);
        if(list == null)
        {
            list = new TreeList();
            keyLockMap.put(key, list);
        }
        if(!list.contains(lock)) list.add(lock);
    }

    private void doReleaseLocks(Object key)
    {
        synchronized(resourceLockMap)
        {
            List list = (List) keyLockMap.get(key);
            if(list != null)
            {
                for(int i = 0; i < list.size(); i++)
                {
                    ObjectLocks lock = (ObjectLocks) list.get(i);
                    lock.releaseLock(key);
                    if(lock.isEmpty()) 
resourceLockMap.remove(lock.getResourceId());
                }
                keyLockMap.remove(key);
            }
        }
    }


    //===============================================================
    // inner class
    //===============================================================
    private static final class ObjectLocks implements Serializable
    {
        private LockEntry writer;
        private Map readers;
        private Object resourceId;
        private long lastReader = 0;
        private long eldestReader = Long.MAX_VALUE;

        ObjectLocks(Object resourceId)
        {
            this.resourceId = resourceId;
        }

        private void newReaderMap()
        {
            this.readers = new HashMap();
        }

        boolean isEmpty()
        {
            return writer == null && (readers == null || readers.size() == 0);
        }

        boolean releaseLock(Object key)
        {
            if(writer != null && writer.isOwnedBy(key))
            {
                writer = null;
                return true;
            }
            else if(readers != null)
            {
                return readers.remove(key) != null;
            }
            return false;
        }

        public Object getResourceId()
        {
            return resourceId;
        }

        boolean addWriter(LockEntry writer, boolean allowReaders)
        {
            boolean result = false;
            if(this.writer != null)
            {
                result = this.writer.isOwnedBy(writer.getKey());
            }
            else // writer was not set
            {
                // current ObjectLock has no write lock, so check for readers
                int readerSize = readers != null ? readers.size() : 0;
                if(readerSize > 0)
                {
                    // does current entity have already an read lock
                    if(getReader(writer.getKey()) != null)
                    {
                        if(readerSize == 1)
                        {
                            // only current entity has a read lock, so go on
                            setWriter(writer);
                            removeReader(this.writer.getKey());
                            result = true;
                        }
                        else
                        {
                            // current entity and others have already a read 
lock
                            // if aquire a write is allowed, go on
                            if(allowReaders)
                            {
                                setWriter(writer);
                                removeReader(this.writer.getKey());
                                result = true;
                            }
                        }
                    }
                    else
                    {
                        // current entity has no read lock, but others
                        // if aquire a write is allowed, go on
                        if(allowReaders)
                        {
                            setWriter(writer);
                            result = true;
                        }
                    }
                }
                else
                {
                    setWriter(writer);
                    result = true;
                }
            }
            return result;
        }

        LockEntry getWriter()
        {
            return this.writer;
        }

        boolean hasWriteLock(Object key)
        {
            return writer != null && writer.isOwnedBy(key);
        }

        private void setWriter(LockEntry writer)
        {
            this.writer = writer;
            if(this.writer != null) this.resourceId = writer.getResourceId();
        }

        Map getReaders()
        {
            return readers;
        }

        boolean hasReadLock(Object key)
        {
            if(writer != null)
            {
                return writer.isOwnedBy(key);
            }
            else if(readers != null)
            {
                LockEntry lock = (LockEntry) readers.get(key);
                return lock != null && lock.isOwnedBy(key);
            }
            return false;
        }

        boolean addReader(LockEntry reader, boolean allowMultipleReader, 
boolean allowReaderWhenWriteLock)
        {
            if(writer == null)
            {
                return internalAddReader(reader, allowMultipleReader);
            }
            else if(writer.isOwnedBy(reader.getKey()))
            {
                return true;
            }
            else if(allowReaderWhenWriteLock)
            {
                return internalAddReader(reader, allowMultipleReader);
            }
            return false;
        }

        private boolean internalAddReader(LockEntry reader, boolean 
allowMultipleReader)
        {
            boolean result = true;
            if(readers == null)
            {
                newReaderMap();
                readers.put(reader.getKey(), reader);
            }
            else if(readers.size() == 0)
            {
                readers.put(reader.getKey(), reader);
            }
            else
            {
                if(this.readers.get(reader.getKey()) != null)
                {
                }
                else if(allowMultipleReader)
                {
                    this.readers.put(reader.getKey(), reader);
                }
                else
                {
                    result = false;
                }
            }
            if(result)
            {
                if(reader.timestamp > lastReader) lastReader = reader.timestamp;
                if(reader.timestamp < eldestReader) eldestReader = 
reader.timestamp;
            }
            return result;
        }

        LockEntry getReader(Object key)
        {
            return this.readers != null ? (LockEntry) this.readers.get(key) : 
null;
        }

        LockEntry removeReader(Object key)
        {
            return this.readers != null ? (LockEntry) this.readers.remove(key) 
: null;
        }
    }

    //===============================================================
    // inner class
    //===============================================================

    /** A lock entry encapsulates locking information. */
    private static final class LockEntry implements Serializable
    {
        /** marks a Read Lock. */
        static final int LOCK_READ = 0;

        /** marks a Write Lock. */
        static final int LOCK_WRITE = 1;

        /** the object to be locked. */
        private Object resourceId;

        /** key for locked object */
        private Object key;

        /** the timestamp marking the time of acquisition of this lock */
        private long timestamp;

        /**
         * marks if this is a read or a write lock.
         * LOCK_READ = 0;
         * LOCK_WRITE = 1;
         */
        private int lockType;

        /** Multiargument constructor for fast loading of LockEntries by OJB. */
        public LockEntry(Object resourceId,
                         Object key,
                         long timestamp,
                         int lockType)
        {
            this.resourceId = resourceId;
            this.key = key;
            this.timestamp = timestamp;
            this.lockType = lockType;

        }

        /** Returns the resource id of the locked object (or the locked object 
itself). */
        public Object getResourceId()
        {
            return resourceId;
        }

        /** Returns lock key. */
        public Object getKey()
        {
            return key;
        }

        /** returns the timestamp of the acqusition of the lock. */
        public long getTimestamp()
        {
            return timestamp;
        }

        /**
         * returns the locktype of this lock.
         *
         * @return LOCK_READ if lock is a readlock,
         *         LOCK_WRITE if lock is a Write lock.
         */
        public int getLockType()
        {
            return lockType;
        }

        /**
         * sets the locktype of this lockentry.
         *
         * @param locktype LOCK_READ for read, LOCK_WRITE for write lock.
         */
        public void setLockType(int locktype)
        {
            this.lockType = locktype;
        }

        /** Returns true if this lock is owned by the specified key. */
        public boolean isOwnedBy(Object key)
        {
            return this.getKey().equals(key);
        }

        /**
         * Sets the resourceId.
         *
         * @param resourceId The resourceId to set
         */
        public void setresourceId(String resourceId)
        {
            this.resourceId = resourceId;
        }

        /**
         * Sets the timestamp.
         *
         * @param timestamp The timestamp to set
         */
        public void setTimestamp(long timestamp)
        {
            this.timestamp = timestamp;
        }

        /**
         * Sets the key.
         *
         * @param key The key to set
         */
        public void setKey(Object key)
        {
            this.key = key;
        }
    }
}



------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to