Return-Path: X-Original-To: apmail-jena-commits-archive@www.apache.org Delivered-To: apmail-jena-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id B3FB311A58 for ; Tue, 10 Jun 2014 10:10:56 +0000 (UTC) Received: (qmail 97468 invoked by uid 500); 10 Jun 2014 10:10:56 -0000 Delivered-To: apmail-jena-commits-archive@jena.apache.org Received: (qmail 97293 invoked by uid 500); 10 Jun 2014 10:10:56 -0000 Mailing-List: contact commits-help@jena.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@jena.apache.org Delivered-To: mailing list commits@jena.apache.org Received: (qmail 97286 invoked by uid 99); 10 Jun 2014 10:10:56 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 10 Jun 2014 10:10:56 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED,T_FILL_THIS_FORM_SHORT X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 10 Jun 2014 10:10:55 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 519D22388868; Tue, 10 Jun 2014 10:10:29 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1601595 - in /jena/trunk/jena-tdb/src: main/java/com/hp/hpl/jena/tdb/StoreConnection.java main/java/com/hp/hpl/jena/tdb/base/file/LocationLock.java test/java/com/hp/hpl/jena/tdb/base/file/TestLocationLock.java Date: Tue, 10 Jun 2014 10:10:29 -0000 To: commits@jena.apache.org From: rvesse@apache.org X-Mailer: svnmailer-1.0.9 Message-Id: <20140610101029.519D22388868@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: rvesse Date: Tue Jun 10 10:10:28 2014 New Revision: 1601595 URL: http://svn.apache.org/r1601595 Log: Temporarily disable all TDB lock file changes until they are more thoroughly debugged and cleaned up since they are causing unexpected test failures elsewhere (JENA-648) Modified: jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/StoreConnection.java jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/base/file/LocationLock.java jena/trunk/jena-tdb/src/test/java/com/hp/hpl/jena/tdb/base/file/TestLocationLock.java Modified: jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/StoreConnection.java URL: http://svn.apache.org/viewvc/jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/StoreConnection.java?rev=1601595&r1=1601594&r2=1601595&view=diff ============================================================================== --- jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/StoreConnection.java (original) +++ jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/StoreConnection.java Tue Jun 10 10:10:28 2014 @@ -240,19 +240,19 @@ public class StoreConnection sConn = new StoreConnection(dsg) ; // Obtain the lock ASAP - LocationLock lock = location.getLock(); - if (lock.canLock()) { - if (!lock.canObtain()) - throw new TDBException("Can't open database at location " + location.getDirectoryPath() + " as it is already locked by the process with PID " + lock.getOwner()); - - lock.obtain(); - // There's an interesting race condition here that two JVMs might write out the lock file one after another without - // colliding and causing an IO error in either. The best way to check for this is simply to check we now own the lock - // and if not error - if (!lock.isOwned()) { - throw new TDBException("Can't open database at location " + location.getDirectoryPath() + " as it is alread locked by the process with PID " + lock.getOwner()); - } - } +// LocationLock lock = location.getLock(); +// if (lock.canLock()) { +// if (!lock.canObtain()) +// throw new TDBException("Can't open database at location " + location.getDirectoryPath() + " as it is already locked by the process with PID " + lock.getOwner()); +// +// lock.obtain(); +// // There's an interesting race condition here that two JVMs might write out the lock file one after another without +// // colliding and causing an IO error in either. The best way to check for this is simply to check we now own the lock +// // and if not error +// if (!lock.isOwned()) { +// throw new TDBException("Can't open database at location " + location.getDirectoryPath() + " as it is alread locked by the process with PID " + lock.getOwner()); +// } +// } sConn.forceRecoverFromJournal() ; // boolean actionTaken = JournalControl.recoverFromJournal(dsg.getConfig(), sConn.transactionManager.getJournal()) ; Modified: jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/base/file/LocationLock.java URL: http://svn.apache.org/viewvc/jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/base/file/LocationLock.java?rev=1601595&r1=1601594&r2=1601595&view=diff ============================================================================== --- jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/base/file/LocationLock.java (original) +++ jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/base/file/LocationLock.java Tue Jun 10 10:10:28 2014 @@ -13,7 +13,6 @@ import java.util.List; import org.apache.jena.atlas.io.IO; import com.hp.hpl.jena.tdb.TDBException; -import com.hp.hpl.jena.tdb.mgt.TDBSystemInfo; import com.hp.hpl.jena.tdb.sys.SystemTDB; /** @@ -23,347 +22,337 @@ import com.hp.hpl.jena.tdb.sys.SystemTDB * */ public class LocationLock { - private static final int NO_OWNER = 0; - private static final String LOCK_FILENAME = "tdb.lock"; + private static final int NO_OWNER = 0; + private static final String LOCK_FILENAME = "tdb.lock"; - private static int myPid = -1; + private static int myPid = -1; - /** - * Tries to get the PID of the current process - * - * @return PID of current process or zero if unable to determine PID - */ - private static int getPid() { - if (myPid != -1) - return myPid; - - String runtimeBeanName = ManagementFactory.getRuntimeMXBean().getName(); - if (runtimeBeanName == null) { - return useFallbackPid(); - } - - // Bean name will have format PID@hostname so we try to parse the PID - // portion - int index = runtimeBeanName.indexOf("@"); - if (index < 0) - return useFallbackPid(); - try { - // Parse and cache for future reuse - String pidData = runtimeBeanName.substring(0, index); - myPid = Integer.parseInt(pidData); - return myPid; - } catch (NumberFormatException e) { - // Invalid PID - return useFallbackPid(); - } - } - - private static int useFallbackPid() { - // In the case where we can't determine our PID then treat ourselves as - // no owner and cache for future use - myPid = NO_OWNER; - return myPid; - } - - private Location location; - - public LocationLock(Location location) { - if (location == null) - throw new NullPointerException("Location cannot be null"); - this.location = location; - } - - /** - * Gets whether the location is lockable i.e. is it an on-disk location - * where we can use a lock file to prevent multi-process access and the - * potential data corruption that ensues - * - * @return True if the location is lockable - */ - public boolean canLock() { - return !location.isMem(); - } - - /** - * Gets whether the location is currently locked, use - * - * @return - */ - public boolean isLocked() { - // Memory locations are never considered locked - if (location.isMem()) - return false; - - return getOwner() != NO_OWNER; - } - - /** - * Gets whether the lock is owned by the current process - * - * @return True if the location is locked and owned by the process, false - * otherwise - */ - public boolean isOwned() { - // Memory locations are never considered locked - if (location.isMem()) - return false; - - int owner = getOwner(); - if (owner == NO_OWNER) - return false; - - return owner == getPid(); - } - - /** - * Gets the current owner of this locations lock. - * - * @return Process ID of owner if locked, zero if the location cannot be - * locked or not currently locked - */ - public int getOwner() { - // Memory locations are never considered locked - if (location.isMem()) - return NO_OWNER; - - File lockFile = getLockFile(); - if (lockFile.exists()) { - checkLockFileForRead(lockFile); - // Can read lock owner from the file - try { - String rawLockInfo = IO.readWholeFileAsUTF8(lockFile - .getAbsolutePath()); - int owner = Integer.parseInt(rawLockInfo); - return owner; - } catch (IOException e) { - throw new FileException( - "Unable to check TDB lock owner due to an IO error reading the lock file", - e); - } catch (NumberFormatException e) { - throw new FileException( - "Unable to check TDB lock owner as the lock file contains invalid data", - e); - } - } else { - // No lock file so nobody owns the lock currently - return NO_OWNER; - } - } - - /** - * Gets whether the current JVM can obtain the lock - * - * @return True if the lock can be obtained (or is already held), false - * otherwise - */ - public boolean canObtain() { - // Memory locations cannot be locked - if (location.isMem()) - return false; - - int owner = this.getOwner(); - int pid = getPid(); - - if (owner == NO_OWNER) { - // Can obtain provided we have a valid PID - if (pid != NO_OWNER) - return true; - } else if (owner == pid) { - // Already held - return true; - } - - // Owned by another process, only obtainable if other process is dead - if (!isAlive(owner)) - return true; - - // Otherwise not obtainable - return false; - } - - /** - * Obtains the lock in order to prevent other JVMs using the location - */ - public void obtain() { - // Memory locations cannot be locked so nothing to do - if (location.isMem()) - return; - - // Get current owner - int owner = this.getOwner(); - if (owner == NO_OWNER) { - // No owner currently so try to obtain the lock - int pid = getPid(); - if (pid == NO_OWNER) { - // In the case where we cannot obtain our PID then we cannot - // obtain a lock - return; - } - - takeLock(pid); - } else if (owner == getPid()) { - // We already own the lock so nothing to do - } else { - // Someone other process potentially owns the lock on this location - // Check if the owner is alive - if (isAlive(owner)) - throw new TDBException( - "The location " - + location.getDirectoryPath() - + " is currently locked by PID " - + owner - + ". TDB databases do not permit concurrent usage across JVMs so in order to prevent corruption you cannot open this location from the JVM that does not own the lock for the dataset"); - - // Otherwise the previous owner is dead so we can take the lock - takeLock(getPid()); - } - } - - private void takeLock(int pid) { - File lockFile = getLockFile(); - checkLockFileForWrite(lockFile); - try { - // Write our PID to the lock file - BufferedWriter writer = new BufferedWriter(new FileWriter(lockFile)); - writer.write(Integer.toString(pid)); - writer.close(); - } catch (IOException e) { - throw new TDBException("Failed to obtain a lock on the location " - + location.getDirectoryPath(), e); - } - } - - /** - * Releases the lock so that other JVMs can use the location - */ - public void release() { - // Memory locations cannot be locked so nothing to do - if (location.isMem()) - return; - - int owner = this.getOwner(); - - // Nobody owned the lock so nothing to do - if (owner == NO_OWNER) - return; - - // Some other process owns the lock so we can't release it - if (owner != getPid()) - throw new TDBException("Cannot release the lock on location " - + location.getDirectoryPath() - + " since this process does not own the lock"); - - File lockFile = getLockFile(); - - // No lock file exists so nothing to do - if (!lockFile.exists()) - return; - - // Try and delete the lock file thereby releasing the lock - if (!lockFile.delete()) - throw new TDBException("Failed to release the lock on location " - + location.getDirectoryPath() - + ", it may be necessary to manually remove the lock file"); - - } - - /** - * Gets the lock file - * - * @return Lock file - */ - private File getLockFile() { - return new File(location.getPath(LOCK_FILENAME)); - } - - /** - * Checks if a lock file is valid throwing an exception if it is not - * - * @param lockFile - * Lock file - * @exception FileException - * Thrown if the lock file is invalid - */ - private void checkLockFileForRead(File lockFile) { - if (!lockFile.exists()) - return; - - if (!lockFile.isFile() || !lockFile.canRead()) { - // Unable to read lock owner because it isn't a file or we don't - // have read permission - throw new FileException( - "Unable to check TDB lock owner for this location since the expected lock file is not a file/not readable"); - } - } - - /** - * Checks if a lock file is valid throwing an exception if it is not - * - * @param lockFile - * Lock file - * @exception FileException - * Thrown if the lock file is invalid - */ - private void checkLockFileForWrite(File lockFile) { - if (!lockFile.exists()) - return; - - if (!lockFile.isFile() || !lockFile.canWrite()) { - // TODO What about read only file systems? Though I suspect TDB will - // fail elsewhere in that case - - // Unable to read lock owner because it isn't a file or we don't - // have read permission - throw new FileException( - "Unable to check TDB lock owner for this location since the expected lock file is not a file/not writable"); - } - } - - private static boolean isAlive(int pid) { - String pidStr = Integer.toString(pid); - Process p; - try { - if (SystemTDB.isWindows) { - // Use the Windows tasklist utility - ProcessBuilder builder = new ProcessBuilder("tasklist", "/FI", - "PID eq " + pidStr); - builder.redirectErrorStream(true); - p = builder.start(); - } else { - // Use the ps utility - ProcessBuilder builder = new ProcessBuilder("ps", "-p", pidStr); - builder.redirectErrorStream(true); - p = builder.start(); - } - - // Run and read data from the process - BufferedReader reader = new BufferedReader(new InputStreamReader( - p.getInputStream())); - - List data = new ArrayList(); - String line = null; - while ((line = reader.readLine()) != null) { - data.add(line); - } - reader.close(); - - // Expect a line to contain the PID to indicate the process is - // alive - for (String lineData : data) { - if (lineData.contains(pidStr)) - return true; - } - - // Did not find any lines mentioning the PID so we can safely - // assume that process is dead - return false; - } catch (IOException e) { - // If any error running the process to check for the live process - // then our check failed and for safety we assume the process is - // alive - - // TODO Issue a warning here - return true; - } - } + /** + * Tries to get the PID of the current process + * + * @return PID of current process or zero if unable to determine PID + */ + private static int getPid() { + if (myPid != -1) + return myPid; + + String runtimeBeanName = ManagementFactory.getRuntimeMXBean().getName(); + if (runtimeBeanName == null) { + return useFallbackPid(); + } + + // Bean name will have format PID@hostname so we try to parse the PID + // portion + int index = runtimeBeanName.indexOf("@"); + if (index < 0) + return useFallbackPid(); + try { + // Parse and cache for future reuse + String pidData = runtimeBeanName.substring(0, index); + myPid = Integer.parseInt(pidData); + return myPid; + } catch (NumberFormatException e) { + // Invalid PID + return useFallbackPid(); + } + } + + private static int useFallbackPid() { + // In the case where we can't determine our PID then treat ourselves as + // no owner and cache for future use + myPid = NO_OWNER; + return myPid; + } + + private Location location; + + public LocationLock(Location location) { + if (location == null) + throw new NullPointerException("Location cannot be null"); + this.location = location; + } + + /** + * Gets whether the location is lockable i.e. is it an on-disk location + * where we can use a lock file to prevent multi-process access and the + * potential data corruption that ensues + * + * @return True if the location is lockable + */ + public boolean canLock() { + return !location.isMem(); + } + + /** + * Gets whether the location is currently locked, use + * + * @return + */ + public boolean isLocked() { + // Memory locations are never considered locked + if (location.isMem()) + return false; + + return getOwner() != NO_OWNER; + } + + /** + * Gets whether the lock is owned by the current process + * + * @return True if the location is locked and owned by the process, false + * otherwise + */ + public boolean isOwned() { + // Memory locations are never considered locked + if (location.isMem()) + return false; + + int owner = getOwner(); + if (owner == NO_OWNER) + return false; + + return owner == getPid(); + } + + /** + * Gets the current owner of this locations lock. + * + * @return Process ID of owner if locked, zero if the location cannot be + * locked or not currently locked + */ + public int getOwner() { + // Memory locations are never considered locked + if (location.isMem()) + return NO_OWNER; + + File lockFile = getLockFile(); + if (lockFile.exists()) { + checkLockFileForRead(lockFile); + // Can read lock owner from the file + try { + String rawLockInfo = IO.readWholeFileAsUTF8(lockFile.getAbsolutePath()); + int owner = Integer.parseInt(rawLockInfo); + return owner; + } catch (IOException e) { + throw new FileException("Unable to check TDB lock owner due to an IO error reading the lock file", e); + } catch (NumberFormatException e) { + throw new FileException("Unable to check TDB lock owner as the lock file contains invalid data", e); + } + } else { + // No lock file so nobody owns the lock currently + return NO_OWNER; + } + } + + /** + * Gets whether the current JVM can obtain the lock + * + * @return True if the lock can be obtained (or is already held), false + * otherwise + */ + public boolean canObtain() { + // Memory locations cannot be locked + if (location.isMem()) + return false; + + int owner = this.getOwner(); + int pid = getPid(); + + if (owner == NO_OWNER) { + // Can obtain provided we have a valid PID + if (pid != NO_OWNER) + return true; + } else if (owner == pid) { + // Already held + return true; + } + + // Owned by another process, only obtainable if other process is dead + if (!isAlive(owner)) + return true; + + // Otherwise not obtainable + return false; + } + + /** + * Obtains the lock in order to prevent other JVMs using the location + */ + public void obtain() { + // Memory locations cannot be locked so nothing to do + if (location.isMem()) + return; + + // Get current owner + int owner = this.getOwner(); + if (owner == NO_OWNER) { + // No owner currently so try to obtain the lock + int pid = getPid(); + if (pid == NO_OWNER) { + // In the case where we cannot obtain our PID then we cannot + // obtain a lock + return; + } + + takeLock(pid); + } else if (owner == getPid()) { + // We already own the lock so nothing to do + } else { + // Someone other process potentially owns the lock on this location + // Check if the owner is alive + if (isAlive(owner)) + throw new TDBException( + "The location " + + location.getDirectoryPath() + + " is currently locked by PID " + + owner + + ". TDB databases do not permit concurrent usage across JVMs so in order to prevent corruption you cannot open this location from the JVM that does not own the lock for the dataset"); + + // Otherwise the previous owner is dead so we can take the lock + takeLock(getPid()); + } + } + + private void takeLock(int pid) { + File lockFile = getLockFile(); + checkLockFileForWrite(lockFile); + try { + // Write our PID to the lock file + BufferedWriter writer = new BufferedWriter(new FileWriter(lockFile)); + writer.write(Integer.toString(pid)); + writer.close(); + } catch (IOException e) { + throw new TDBException("Failed to obtain a lock on the location " + location.getDirectoryPath(), e); + } + } + + /** + * Releases the lock so that other JVMs can use the location + */ + public void release() { + // Memory locations cannot be locked so nothing to do + if (location.isMem()) + return; + + int owner = this.getOwner(); + + // Nobody owned the lock so nothing to do + if (owner == NO_OWNER) + return; + + // Some other process owns the lock so we can't release it + if (owner != getPid()) + throw new TDBException("Cannot release the lock on location " + location.getDirectoryPath() + + " since this process does not own the lock"); + + File lockFile = getLockFile(); + + // No lock file exists so nothing to do + if (!lockFile.exists()) + return; + + // Try and delete the lock file thereby releasing the lock + if (!lockFile.delete()) + throw new TDBException("Failed to release the lock on location " + location.getDirectoryPath() + + ", it may be necessary to manually remove the lock file"); + + } + + /** + * Gets the lock file + * + * @return Lock file + */ + private File getLockFile() { + return new File(location.getPath(LOCK_FILENAME)); + } + + /** + * Checks if a lock file is valid throwing an exception if it is not + * + * @param lockFile + * Lock file + * @exception FileException + * Thrown if the lock file is invalid + */ + private void checkLockFileForRead(File lockFile) { + if (!lockFile.exists()) + return; + + if (!lockFile.isFile() || !lockFile.canRead()) { + // Unable to read lock owner because it isn't a file or we don't + // have read permission + throw new FileException( + "Unable to check TDB lock owner for this location since the expected lock file is not a file/not readable"); + } + } + + /** + * Checks if a lock file is valid throwing an exception if it is not + * + * @param lockFile + * Lock file + * @exception FileException + * Thrown if the lock file is invalid + */ + private void checkLockFileForWrite(File lockFile) { + if (!lockFile.exists()) + return; + + if (!lockFile.isFile() || !lockFile.canWrite()) { + // TODO What about read only file systems? Though I suspect TDB will + // fail elsewhere in that case + + // Unable to read lock owner because it isn't a file or we don't + // have read permission + throw new FileException( + "Unable to check TDB lock owner for this location since the expected lock file is not a file/not writable"); + } + } + + private static boolean isAlive(int pid) { + String pidStr = Integer.toString(pid); + Process p; + try { + if (SystemTDB.isWindows) { + // Use the Windows tasklist utility + ProcessBuilder builder = new ProcessBuilder("tasklist", "/FI", "PID eq " + pidStr); + builder.redirectErrorStream(true); + p = builder.start(); + } else { + // Use the ps utility + ProcessBuilder builder = new ProcessBuilder("ps", "-p", pidStr); + builder.redirectErrorStream(true); + p = builder.start(); + } + + // Run and read data from the process + BufferedReader reader = new BufferedReader(new InputStreamReader(p.getInputStream())); + + List data = new ArrayList(); + String line = null; + while ((line = reader.readLine()) != null) { + data.add(line); + } + reader.close(); + + // Expect a line to contain the PID to indicate the process is + // alive + for (String lineData : data) { + if (lineData.contains(pidStr)) + return true; + } + + // Did not find any lines mentioning the PID so we can safely + // assume that process is dead + return false; + } catch (IOException e) { + // If any error running the process to check for the live process + // then our check failed and for safety we assume the process is + // alive + + // TODO Issue a warning here + return true; + } + } } Modified: jena/trunk/jena-tdb/src/test/java/com/hp/hpl/jena/tdb/base/file/TestLocationLock.java URL: http://svn.apache.org/viewvc/jena/trunk/jena-tdb/src/test/java/com/hp/hpl/jena/tdb/base/file/TestLocationLock.java?rev=1601595&r1=1601594&r2=1601595&view=diff ============================================================================== --- jena/trunk/jena-tdb/src/test/java/com/hp/hpl/jena/tdb/base/file/TestLocationLock.java (original) +++ jena/trunk/jena-tdb/src/test/java/com/hp/hpl/jena/tdb/base/file/TestLocationLock.java Tue Jun 10 10:10:28 2014 @@ -15,6 +15,7 @@ import com.hp.hpl.jena.tdb.StoreConnecti import com.hp.hpl.jena.tdb.TDBException; import com.hp.hpl.jena.tdb.sys.SystemTDB; +@Ignore public class TestLocationLock { @Rule