QPID-2051: Make the LoggingManagementMBean share the lock with QpidLog4JConfigurator,protecting against concurrent access/modifications to the logging configuration file by JMX clients and the WatchDog thread.

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@804766 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 379 insertions, 313 deletions

diff --git a/qpid/java/broker/src/main/java/org/apache/log4j/xml/QpidLog4JConfigurator.java b/qpid/java/broker/src/main/java/org/apache/log4j/xml/QpidLog4JConfigurator.java
index 53134250ab..aa6694b3b1 100644
--- a/qpid/java/broker/src/main/java/org/apache/log4j/xml/QpidLog4JConfigurator.java
+++ b/qpid/java/broker/src/main/java/org/apache/log4j/xml/QpidLog4JConfigurator.java
@@ -47,7 +47,8 @@ import org.xml.sax.SAXException;
 public class QpidLog4JConfigurator
 {
     //lock to protect access to the configuration file
-    private static final ReentrantLock LOCK = new ReentrantLock();
+    //shared with LoggingManagementMBean
+    public static final ReentrantLock LOCK = new ReentrantLock();
     private static Logger _logger;
 
     private QpidLog4JConfigurator()
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/management/LoggingManagementMBean.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/management/LoggingManagementMBean.java
index 396e81e690..e1e6cb49b5 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/management/LoggingManagementMBean.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/management/LoggingManagementMBean.java
@@ -20,6 +20,8 @@
  */
 package org.apache.qpid.server.logging.management;
 
+import static org.apache.log4j.xml.QpidLog4JConfigurator.LOCK;
+
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
@@ -214,22 +216,22 @@ public class LoggingManagementMBean extends AMQManagedObject implements LoggingM
 
     public synchronized boolean setRuntimeRootLoggerLevel(String level)
     {
-            Level newLevel;
-            try
-            {
-                newLevel = getLevel(level);
-            }
-            catch (Exception e)
-            {
-                return false;
-            }
-            
-            _logger.info("Setting RootLogger level to " + level);
-            
-            Logger log = Logger.getRootLogger();
-            log.setLevel(newLevel);
-            
-            return true;
+        Level newLevel;
+        try
+        {
+            newLevel = getLevel(level);
+        }
+        catch (Exception e)
+        {
+            return false;
+        }
+
+        _logger.info("Setting RootLogger level to " + level);
+
+        Logger log = Logger.getRootLogger();
+        log.setLevel(newLevel);
+
+        return true;
     }
     
     //method to convert from a string to a log4j Level, throws exception if the given value is invalid
@@ -274,114 +276,132 @@ public class LoggingManagementMBean extends AMQManagedObject implements LoggingM
     //method to parse the XML configuration file, validating it in the process, and returning a DOM Document of the content.
     private static synchronized Document parseConfigFile(String fileName) throws IOException
     {
-        //check file was specified, exists, and is readable
-        if(fileName == null)
+        try
         {
-            _logger.warn("Provided log4j XML configuration filename is null");
-            throw new IOException("Provided log4j XML configuration filename is null");
-        }
+            LOCK.lock();
 
-        File configFile = new File(fileName);
+            //check file was specified, exists, and is readable
+            if(fileName == null)
+            {
+                _logger.warn("Provided log4j XML configuration filename is null");
+                throw new IOException("Provided log4j XML configuration filename is null");
+            }
 
-        if (!configFile.exists())
-        {
-            _logger.warn("The log4j XML configuration file could not be found: " + fileName);
-            throw new IOException("The log4j XML configuration file could not be found");
-        }
-        else if (!configFile.canRead())
-        {
-            _logger.warn("The log4j XML configuration file is not readable: " + fileName);
-            throw new IOException("The log4j XML configuration file is not readable");
-        }
+            File configFile = new File(fileName);
 
-        //parse it
-        DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance();
-        DocumentBuilder docBuilder;
-        Document doc;
+            if (!configFile.exists())
+            {
+                _logger.warn("The log4j XML configuration file could not be found: " + fileName);
+                throw new IOException("The log4j XML configuration file could not be found");
+            }
+            else if (!configFile.canRead())
+            {
+                _logger.warn("The log4j XML configuration file is not readable: " + fileName);
+                throw new IOException("The log4j XML configuration file is not readable");
+            }
 
-        ErrorHandler errHandler = new QpidLog4JSaxErrorHandler();
-        try
-        {
-            docFactory.setValidating(true);
-            docBuilder = docFactory.newDocumentBuilder();
-            docBuilder.setErrorHandler(errHandler);
-            docBuilder.setEntityResolver(new Log4jEntityResolver());
-            doc = docBuilder.parse(fileName);
-        }
-        catch (ParserConfigurationException e)
-        {
-            _logger.warn("Unable to parse the log4j XML file due to possible configuration error: " + e);
-            //recommended that MBeans should use java.* and javax.* exceptions only
-            throw new IOException("Unable to parse the log4j XML file due to possible configuration error: " + e.getMessage());
-        }
-        catch (SAXException e)
-        {
-            _logger.warn("The specified log4j XML file is invalid: " + e);
-            //recommended that MBeans should use standard java.* and javax.* exceptions only
-            throw new IOException("The specified log4j XML file is invalid: " + e.getMessage());
+            //parse it
+            DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance();
+            DocumentBuilder docBuilder;
+            Document doc;
+
+            ErrorHandler errHandler = new QpidLog4JSaxErrorHandler();
+            try
+            {
+                docFactory.setValidating(true);
+                docBuilder = docFactory.newDocumentBuilder();
+                docBuilder.setErrorHandler(errHandler);
+                docBuilder.setEntityResolver(new Log4jEntityResolver());
+                doc = docBuilder.parse(fileName);
+            }
+            catch (ParserConfigurationException e)
+            {
+                _logger.warn("Unable to parse the log4j XML file due to possible configuration error: " + e);
+                //recommended that MBeans should use java.* and javax.* exceptions only
+                throw new IOException("Unable to parse the log4j XML file due to possible configuration error: " + e.getMessage());
+            }
+            catch (SAXException e)
+            {
+                _logger.warn("The specified log4j XML file is invalid: " + e);
+                //recommended that MBeans should use standard java.* and javax.* exceptions only
+                throw new IOException("The specified log4j XML file is invalid: " + e.getMessage());
+            }
+            catch (IOException e)
+            {
+                _logger.warn("Unable to parse the specified log4j XML file" + e);
+                throw new IOException("Unable to parse the specified log4j XML file: " + e.getMessage());
+            }
+
+            return doc;
         }
-        catch (IOException e)
+        finally
         {
-            _logger.warn("Unable to parse the specified log4j XML file" + e);
-            throw new IOException("Unable to parse the specified log4j XML file: " + e.getMessage());
+            LOCK.unlock();
         }
-
-        return doc;
     }
 
     
     private static synchronized boolean writeUpdatedConfigFile(String log4jConfigFileName, Document doc) throws IOException
     {
-        File log4jConfigFile = new File(log4jConfigFileName);
-        
-        if (!log4jConfigFile.canWrite())
-        {
-            _logger.warn("Specified log4j XML configuration file is not writable: " + log4jConfigFile);
-            throw new IOException("Specified log4j XML configuration file is not writable");
-        }
-
-        Transformer transformer = null;
         try
         {
-            transformer = TransformerFactory.newInstance().newTransformer();
-        }
-        catch (Exception e)
-        {
-            _logger.warn("Could not create an XML transformer: " +e);
-            return false;
-        }
+            LOCK.lock();
 
-        transformer.setOutputProperty(OutputKeys.INDENT, "yes");
-        transformer.setOutputProperty(OutputKeys.DOCTYPE_SYSTEM, "log4j.dtd");
-        DOMSource source = new DOMSource(doc);
-        
-        File tmp;
-        try
-        {
-            tmp = File.createTempFile("LogManMBeanTemp", ".tmp");
-            tmp.deleteOnExit();
-            StreamResult result = new StreamResult(tmp);
-            transformer.transform(source, result);
-        }
-        catch (TransformerException e)
-        {
-            _logger.warn("Could not transform the XML into new file: " +e);
-            return false;
-        }
-        catch (IOException e)
-        {
-            _logger.warn("Could not create the new file: " +e);
-            return false;
-        }
+            File log4jConfigFile = new File(log4jConfigFileName);
+
+            if (!log4jConfigFile.canWrite())
+            {
+                _logger.warn("Specified log4j XML configuration file is not writable: " + log4jConfigFile);
+                throw new IOException("Specified log4j XML configuration file is not writable");
+            }
+
+            Transformer transformer = null;
+            try
+            {
+                transformer = TransformerFactory.newInstance().newTransformer();
+            }
+            catch (Exception e)
+            {
+                _logger.warn("Could not create an XML transformer: " +e);
+                return false;
+            }
 
-        // Swap temp file in to replace existing configuration file.
-        File old = new File(log4jConfigFile.getAbsoluteFile() + ".old");
-        if (old.exists())
+            transformer.setOutputProperty(OutputKeys.INDENT, "yes");
+            transformer.setOutputProperty(OutputKeys.DOCTYPE_SYSTEM, "log4j.dtd");
+            DOMSource source = new DOMSource(doc);
+
+            File tmp;
+            try
+            {
+                tmp = File.createTempFile("LogManMBeanTemp", ".tmp");
+                tmp.deleteOnExit();
+                StreamResult result = new StreamResult(tmp);
+                transformer.transform(source, result);
+            }
+            catch (TransformerException e)
+            {
+                _logger.warn("Could not transform the XML into new file: " +e);
+                return false;
+            }
+            catch (IOException e)
+            {
+                _logger.warn("Could not create the new file: " +e);
+                return false;
+            }
+
+            // Swap temp file in to replace existing configuration file.
+            File old = new File(log4jConfigFile.getAbsoluteFile() + ".old");
+            if (old.exists())
+            {
+                old.delete();
+            }
+            log4jConfigFile.renameTo(old);
+            return tmp.renameTo(log4jConfigFile);
+        }
+        finally
         {
-            old.delete();
+            LOCK.unlock();
         }
-        log4jConfigFile.renameTo(old);
-        return tmp.renameTo(log4jConfigFile);
     }
 
 
@@ -399,180 +419,207 @@ public class LoggingManagementMBean extends AMQManagedObject implements LoggingM
     
     public static synchronized Map<String,String> retrieveConfigFileLoggersLevels(String fileName) throws IOException
     {
-        Document doc = parseConfigFile(fileName);
+        try
+        {
+            LOCK.lock();
 
-        HashMap<String,String> loggerLevelList = new HashMap<String,String>();
+            Document doc = parseConfigFile(fileName);
 
-        //retrieve the 'category' element nodes
-        NodeList categoryElements = doc.getElementsByTagName("category");
-        
-        String categoryName;
-        String priority = null;
-        
-        for (int i = 0; i < categoryElements.getLength(); i++)
-        {
-            Element categoryElement = (Element) categoryElements.item(i);
-            categoryName = categoryElement.getAttribute("name");
+            HashMap<String,String> loggerLevelList = new HashMap<String,String>();
 
-            //retrieve the category's mandatory 'priority' or 'level' element's value.
-            //It may not be the only child node, so request by tag name.
-            NodeList priorityElements = categoryElement.getElementsByTagName("priority");
-            NodeList levelElements = categoryElement.getElementsByTagName("level");
+            //retrieve the 'category' element nodes
+            NodeList categoryElements = doc.getElementsByTagName("category");
 
-            if (priorityElements.getLength() != 0)
+            String categoryName;
+            String priority = null;
+
+            for (int i = 0; i < categoryElements.getLength(); i++)
             {
-                Element priorityElement = (Element) priorityElements.item(0);
-                priority = priorityElement.getAttribute("value").toUpperCase();
+                Element categoryElement = (Element) categoryElements.item(i);
+                categoryName = categoryElement.getAttribute("name");
+
+                //retrieve the category's mandatory 'priority' or 'level' element's value.
+                //It may not be the only child node, so request by tag name.
+                NodeList priorityElements = categoryElement.getElementsByTagName("priority");
+                NodeList levelElements = categoryElement.getElementsByTagName("level");
+
+                if (priorityElements.getLength() != 0)
+                {
+                    Element priorityElement = (Element) priorityElements.item(0);
+                    priority = priorityElement.getAttribute("value");
+                }
+                else if (levelElements.getLength() != 0)
+                {
+                    Element levelElement = (Element) levelElements.item(0);
+                    priority = levelElement.getAttribute("value");
+                }
+                else
+                {
+                    //there is no exiting priority or level to view, move onto next category/logger
+                    continue;
+                }
+
+                loggerLevelList.put(categoryName, priority);
             }
-            else if (levelElements.getLength() != 0)
+
+            //retrieve the 'logger' element nodes
+            NodeList loggerElements = doc.getElementsByTagName("logger");
+
+            String loggerName;
+            String level;
+
+            for (int i = 0; i < loggerElements.getLength(); i++)
             {
+                Element loggerElement = (Element) loggerElements.item(i);
+                loggerName = loggerElement.getAttribute("name");
+
+                //retrieve the logger's mandatory 'level' element's value
+                //It may not be the only child node, so request by tag name.
+                NodeList levelElements = loggerElement.getElementsByTagName("level");
+
                 Element levelElement = (Element) levelElements.item(0);
-                priority = levelElement.getAttribute("value").toUpperCase();
-            }
-            else
-            {
-                //there is no exiting priority or level to view, move onto next category/logger
-                continue;
+                level = levelElement.getAttribute("value");
+
+                loggerLevelList.put(loggerName, level);
             }
 
-            loggerLevelList.put(categoryName, priority);
+            return loggerLevelList;
         }
-
-        //retrieve the 'logger' element nodes
-        NodeList loggerElements = doc.getElementsByTagName("logger");
-        
-        String loggerName;
-        String level;
-
-        for (int i = 0; i < loggerElements.getLength(); i++)
+        finally
         {
-            Element loggerElement = (Element) loggerElements.item(i);
-            loggerName = loggerElement.getAttribute("name");
-
-            //retrieve the logger's mandatory 'level' element's value
-            //It may not be the only child node, so request by tag name.
-            NodeList levelElements = loggerElement.getElementsByTagName("level");
-
-            Element levelElement = (Element) levelElements.item(0);
-            level = levelElement.getAttribute("value").toUpperCase();
-            
-            loggerLevelList.put(loggerName, level);
+            LOCK.unlock();
         }
-        
-        return loggerLevelList;
     }
 
     public synchronized TabularData viewConfigFileLoggerLevels() throws IOException
     {
-        if (_loggerLevelTabularType == null)
+        try
         {
-            _logger.warn("TabluarData type not set up correctly");
-            return null;
-        }
-        
-        _logger.info("Getting logger levels from log4j configuration file");
+            LOCK.lock();    
 
-        TabularData loggerLevelList = new TabularDataSupport(_loggerLevelTabularType);
-        
-        Map<String,String> levels = retrieveConfigFileLoggersLevels(_log4jConfigFileName);
-
-        for (String loggerName : levels.keySet())
-        {
-            String level = levels.get(loggerName);
-            
-            try
+            if (_loggerLevelTabularType == null)
             {
-                Object[] itemData = {loggerName, level};
-                CompositeData loggerData = new CompositeDataSupport(_loggerLevelCompositeType, COMPOSITE_ITEM_NAMES, itemData);
-                loggerLevelList.put(loggerData);
+                _logger.warn("TabluarData type not set up correctly");
+                return null;
             }
-            catch (OpenDataException e)
+
+            _logger.info("Getting logger levels from log4j configuration file");
+
+            TabularData loggerLevelList = new TabularDataSupport(_loggerLevelTabularType);
+
+            Map<String,String> levels = retrieveConfigFileLoggersLevels(_log4jConfigFileName);
+
+            for (String loggerName : levels.keySet())
             {
-                _logger.warn("Unable to create logger level list due to :" + e);
-                return null;
+                String level = levels.get(loggerName);
+
+                try
+                {
+                    Object[] itemData = {loggerName, level.toUpperCase()};
+                    CompositeData loggerData = new CompositeDataSupport(_loggerLevelCompositeType, COMPOSITE_ITEM_NAMES, itemData);
+                    loggerLevelList.put(loggerData);
+                }
+                catch (OpenDataException e)
+                {
+                    _logger.warn("Unable to create logger level list due to :" + e);
+                    return null;
+                }
             }
+
+            return loggerLevelList;
+        }
+        finally
+        {
+            LOCK.unlock();
         }
-        
-        return loggerLevelList;
     }
 
     public synchronized boolean setConfigFileLoggerLevel(String logger, String level) throws IOException
     {
-        //check that the specified level is a valid log4j Level
         try
         {
-            getLevel(level);
-        }
-        catch (Exception e)
-        {
-            //it isnt a valid level
-            return false;
-        }
-        
-        _logger.info("Setting level to " + level + " for logger '" + logger
-                + "' in log4j xml configuration file: " + _log4jConfigFileName);
-        
-        Document doc = parseConfigFile(_log4jConfigFileName);
+            LOCK.lock();
 
-        //retrieve the 'category' and 'logger' element nodes
-        NodeList categoryElements = doc.getElementsByTagName("category");
-        NodeList loggerElements = doc.getElementsByTagName("logger");
-        
-        //collect them into a single elements list
-        List<Element> logElements = new ArrayList<Element>();
-        
-        for (int i = 0; i < categoryElements.getLength(); i++)
-        {
-            logElements.add((Element) categoryElements.item(i));
-        }
-        for (int i = 0; i < loggerElements.getLength(); i++)
-        {
-            logElements.add((Element) loggerElements.item(i));
-        }
+            //check that the specified level is a valid log4j Level
+            try
+            {
+                getLevel(level);
+            }
+            catch (Exception e)
+            {
+                //it isnt a valid level
+                return false;
+            }
 
-        //try to locate the specified logger/category in the elements retrieved
-        Element logElement = null;
-        for (Element e : logElements)
-        {
-            if (e.getAttribute("name").equals(logger))
+            _logger.info("Setting level to " + level + " for logger '" + logger
+                    + "' in log4j xml configuration file: " + _log4jConfigFileName);
+
+            Document doc = parseConfigFile(_log4jConfigFileName);
+
+            //retrieve the 'category' and 'logger' element nodes
+            NodeList categoryElements = doc.getElementsByTagName("category");
+            NodeList loggerElements = doc.getElementsByTagName("logger");
+
+            //collect them into a single elements list
+            List<Element> logElements = new ArrayList<Element>();
+
+            for (int i = 0; i < categoryElements.getLength(); i++)
             {
-                logElement = e;
-                break;
+                logElements.add((Element) categoryElements.item(i));
+            }
+            for (int i = 0; i < loggerElements.getLength(); i++)
+            {
+                logElements.add((Element) loggerElements.item(i));
             }
-        }
 
-        if (logElement == null)
-        {
-            //no loggers/categories with given name found, does not exist to update
-            _logger.warn("Specified logger does not exist in the configuration file: " +logger);
-            return false;
-        }
+            //try to locate the specified logger/category in the elements retrieved
+            Element logElement = null;
+            for (Element e : logElements)
+            {
+                if (e.getAttribute("name").equals(logger))
+                {
+                    logElement = e;
+                    break;
+                }
+            }
 
-        //retrieve the optional 'priority' or 'level' sub-element value.
-        //It may not be the only child node, so request by tag name.
-        NodeList priorityElements = logElement.getElementsByTagName("priority");
-        NodeList levelElements = logElement.getElementsByTagName("level");
+            if (logElement == null)
+            {
+                //no loggers/categories with given name found, does not exist to update
+                _logger.warn("Specified logger does not exist in the configuration file: " +logger);
+                return false;
+            }
 
-        Element levelElement = null;
-        if (priorityElements.getLength() != 0)
-        {
-            levelElement = (Element) priorityElements.item(0);
-        }
-        else if (levelElements.getLength() != 0)
-        {
-            levelElement = (Element) levelElements.item(0);
+            //retrieve the optional 'priority' or 'level' sub-element value.
+            //It may not be the only child node, so request by tag name.
+            NodeList priorityElements = logElement.getElementsByTagName("priority");
+            NodeList levelElements = logElement.getElementsByTagName("level");
+
+            Element levelElement = null;
+            if (priorityElements.getLength() != 0)
+            {
+                levelElement = (Element) priorityElements.item(0);
+            }
+            else if (levelElements.getLength() != 0)
+            {
+                levelElement = (Element) levelElements.item(0);
+            }
+            else
+            {
+                //there is no exiting priority or level element to update
+                return false;
+            }
+
+            //update the element with the new level/priority
+            levelElement.setAttribute("value", level);
+
+            //output the new file
+            return writeUpdatedConfigFile(_log4jConfigFileName, doc);
         }
-        else
+        finally
         {
-            //there is no exiting priority or level element to update
-            return false;
+            LOCK.unlock();
         }
-        
-        //update the element with the new level/priority
-        levelElement.setAttribute("value", level);
-        
-        //output the new file
-        return writeUpdatedConfigFile(_log4jConfigFileName, doc);
     }
 
     
@@ -589,103 +636,121 @@ public class LoggingManagementMBean extends AMQManagedObject implements LoggingM
     
     public static synchronized String retrieveConfigFileRootLoggerLevel(String fileName) throws IOException
     {
-        Document doc = parseConfigFile(fileName);
-       
-        //retrieve the optional 'root' element node
-        NodeList rootElements = doc.getElementsByTagName("root");
-
-        if (rootElements.getLength() == 0)
+        try
         {
-            //there is no root logger definition
-            return "N/A";
-        }
+            LOCK.lock();
 
-        Element rootElement = (Element) rootElements.item(0);
+            Document doc = parseConfigFile(fileName);
 
-        //retrieve the optional 'priority' or 'level' element value.
-        //It may not be the only child node, so request by tag name.
-        NodeList priorityElements = rootElement.getElementsByTagName("priority");
-        NodeList levelElements = rootElement.getElementsByTagName("level");
-        String priority = null;
-        
-        if (priorityElements.getLength() != 0)
-        {
-            Element priorityElement = (Element) priorityElements.item(0);
-            priority = priorityElement.getAttribute("value");
-        }
-        else if(levelElements.getLength() != 0)
-        {
-            Element levelElement = (Element) levelElements.item(0);
-            priority = levelElement.getAttribute("value");
-        }
+            //retrieve the optional 'root' element node
+            NodeList rootElements = doc.getElementsByTagName("root");
 
-        if(priority != null)
-        {
-            return priority.toUpperCase();
+            if (rootElements.getLength() == 0)
+            {
+                //there is no root logger definition
+                return "N/A";
+            }
+
+            Element rootElement = (Element) rootElements.item(0);
+
+            //retrieve the optional 'priority' or 'level' element value.
+            //It may not be the only child node, so request by tag name.
+            NodeList priorityElements = rootElement.getElementsByTagName("priority");
+            NodeList levelElements = rootElement.getElementsByTagName("level");
+            String priority = null;
+
+            if (priorityElements.getLength() != 0)
+            {
+                Element priorityElement = (Element) priorityElements.item(0);
+                priority = priorityElement.getAttribute("value");
+            }
+            else if(levelElements.getLength() != 0)
+            {
+                Element levelElement = (Element) levelElements.item(0);
+                priority = levelElement.getAttribute("value");
+            }
+
+            if(priority != null)
+            {
+                return priority;
+            }
+            else
+            {
+                return "N/A";
+            }
         }
-        else
+        finally
         {
-            return null;
+            LOCK.unlock();
         }
     }
     
     public synchronized String getConfigFileRootLoggerLevel() throws IOException
     {
-        return retrieveConfigFileRootLoggerLevel(_log4jConfigFileName);
+        return retrieveConfigFileRootLoggerLevel(_log4jConfigFileName).toUpperCase();
     }
     
     public synchronized boolean setConfigFileRootLoggerLevel(String level) throws IOException
     {
-        //check that the specified level is a valid log4j Level
         try
         {
-            getLevel(level);
-        }
-        catch (Exception e)
-        {
-            //it isnt a valid level
-            return false;
-        }
-        
-        _logger.info("Setting level to " + level + " for the Root logger in " +
-        		"log4j xml configuration file: " + _log4jConfigFileName);
+            LOCK.lock();
 
-        Document doc = parseConfigFile(_log4jConfigFileName);
-        
-        //retrieve the optional 'root' element node
-        NodeList rootElements = doc.getElementsByTagName("root");
+            //check that the specified level is a valid log4j Level
+            try
+            {
+                getLevel(level);
+            }
+            catch (Exception e)
+            {
+                //it isnt a valid level
+                return false;
+            }
 
-        if (rootElements.getLength() == 0)
-        {
-            return false;
-        }
+            _logger.info("Setting level to " + level + " for the Root logger in " +
+                    "log4j xml configuration file: " + _log4jConfigFileName);
 
-        Element rootElement = (Element) rootElements.item(0);
+            Document doc = parseConfigFile(_log4jConfigFileName);
 
-        //retrieve the optional 'priority' or 'level' sub-element value.
-        //It may not be the only child node, so request by tag name.
-        NodeList priorityElements = rootElement.getElementsByTagName("priority");
-        NodeList levelElements = rootElement.getElementsByTagName("level");
+            //retrieve the optional 'root' element node
+            NodeList rootElements = doc.getElementsByTagName("root");
 
-        Element levelElement = null;
-        if (priorityElements.getLength() != 0)
-        {
-            levelElement = (Element) priorityElements.item(0);
-        }
-        else if (levelElements.getLength() != 0)
-        {
-            levelElement = (Element) levelElements.item(0);
+            if (rootElements.getLength() == 0)
+            {
+                return false;
+            }
+
+            Element rootElement = (Element) rootElements.item(0);
+
+            //retrieve the optional 'priority' or 'level' sub-element value.
+            //It may not be the only child node, so request by tag name.
+            NodeList priorityElements = rootElement.getElementsByTagName("priority");
+            NodeList levelElements = rootElement.getElementsByTagName("level");
+
+            Element levelElement = null;
+            if (priorityElements.getLength() != 0)
+            {
+                levelElement = (Element) priorityElements.item(0);
+            }
+            else if (levelElements.getLength() != 0)
+            {
+                levelElement = (Element) levelElements.item(0);
+            }
+            else
+            {
+                //there is no exiting priority/level to update
+                return false;
+            }
+
+            //update the element with the new level/priority
+            levelElement.setAttribute("value", level);
+
+            //output the new file
+            return writeUpdatedConfigFile(_log4jConfigFileName, doc);
         }
-        else
+        finally
         {
-            //there is no exiting priority/level to update
-            return false;
+            LOCK.unlock();
         }
-        
-        //update the element with the new level/priority
-        levelElement.setAttribute("value", level);
-
-        //output the new file
-        return writeUpdatedConfigFile(_log4jConfigFileName, doc);
     }
 }