[Exist-commits] eXist-1.0/src/org/exist/collections Collection.java,1.41,1.42

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 422-6466

Update of /cvsroot/exist/eXist-1.0/src/org/exist/collections
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv4115/src/org/exist/collections

Modified Files:
	Collection.java 
Log Message:
* Fixed locking error: a collection object could be unloaded from the cache by one thread, while another thread had still been writing a document 
being a member of the collection. A moment later, a third thread reloaded the collection and started to write to the same document. However, as 
the collection had been recreated, there were two instances of the same document in memory. The third thread did thus not respect the lock held 
by the first thread and started to write without waiting. As a consequence, various page errors were thrown.

* When updating a document, the old document metadata had not been correctly removed. Usually, this had no consequences as the collection class 
only used the last record it found. In some cases however, this may have led to errors.

Index: Collection.java
===================================================================
RCS file: /cvsroot/exist/eXist-1.0/src/org/exist/collections/Collection.java,v
retrieving revision 1.41
retrieving revision 1.42
diff -C2 -d -r1.41 -r1.42
*** Collection.java	13 Jul 2004 15:09:44 -0000	1.41
--- Collection.java	28 Jul 2004 18:54:54 -0000	1.42
***************
*** 130,134 ****
  	private int refCount = 0;
  	private int timestamp = 0;
! 
  	// the collection store where this collections is stored.
  	private CollectionStore db;
--- 130,134 ----
  	private int refCount = 0;
  	private int timestamp = 0;
! 	
  	// the collection store where this collections is stored.
  	private CollectionStore db;
***************
*** 342,347 ****
  		}
  		return true;
  	}
! 
  	public int compareTo(Object obj) {
  		Collection other = (Collection) obj;
--- 342,361 ----
  		}
  		return true;
+ //		try {
+ //			lock.acquire(Lock.WRITE_LOCK);
+ //			for (Iterator i = documents.values().iterator(); i.hasNext(); ) {
+ //				DocumentImpl doc = (DocumentImpl) i.next();
+ //				if (doc.isLockedForWrite())
+ //					return false;
+ //			}
+ //			return true;
+ //		} catch (LockException e) {
+ //			LOG.warn("Failed to acquire lock on collection: " + getName(), e);
+ //		} finally {
+ //			lock.release();
+ //		}
+ //		return false;
  	}
! 	
  	public int compareTo(Object obj) {
  		Collection other = (Collection) obj;
***************
*** 716,728 ****
  		DocumentImpl document, oldDoc = null;
  		XMLReader reader;
! 
! 		InputSource source = new InputSource(new StringReader(data));
! 		oldDoc = getDocument(broker, name);
! 		document = new DocumentImpl(broker, name,	this);
! 		reader = getReader(broker);

- 		// first pass: parse the document to determine tree structure
- 		document = determineTreeStructure(broker, name, document, oldDoc, reader, source);
- 
  		// reset the input source
  		source = new InputSource(new StringReader(data));
--- 730,747 ----
  		DocumentImpl document, oldDoc = null;
  		XMLReader reader;
! 		InputSource source;
! 		try {
! 			lock.acquire(Lock.WRITE_LOCK);
! 			source = new InputSource(new StringReader(data));
! 			document = new DocumentImpl(broker, name,	this);
! 			reader = getReader(broker);
! 			oldDoc = (DocumentImpl) documents.get(name);
! 			
! 			// first pass: parse the document to determine tree structure
! 			document = determineTreeStructure(broker, name, document, oldDoc, reader, source);
! 		} finally {
! 			lock.release();
! 		}

  		// reset the input source
  		source = new InputSource(new StringReader(data));
***************
*** 736,746 ****
  				throw new EXistException(e);
  			}
! 	
! 			if(oldDoc == null)
! 			    addDocument(broker, document);
! //			broker.checkTree(document);
! 			broker.addDocument(this, document);
  			broker.closeDocument();
  			broker.flush();
  			LOG.debug("document stored.");
  			// if we are running in privileged mode (e.g. backup/restore)
--- 755,768 ----
  				throw new EXistException(e);
  			}
! 
! 			if(oldDoc == null) {
! 				addDocument(broker, document);
! 				broker.addDocument(this, document);
! 			} else {
! 				broker.updateDocument(document);
! 			}
  			broker.closeDocument();
  			broker.flush();
+ //			broker.checkTree(document);
  			LOG.debug("document stored.");
  			// if we are running in privileged mode (e.g. backup/restore)
***************
*** 795,806 ****
  			document.setMaxDepth(document.getMaxDepth() + 1);
  			document.calculateTreeLevelStartPoints();
- 			// new document is valid: remove old document

  			if (oldDoc != null) {
  				LOG.debug("removing old document " + oldDoc.getFileName());
  				if (oldDoc.getResourceType() == DocumentImpl.BINARY_FILE)
  					broker.removeBinaryResource((BinaryDocument) oldDoc);
! 				else
  					broker.removeDocument(getName() + '/' + oldDoc.getFileName(), false);
  				// we continue to use the old document object and just replace its contents
  				oldDoc.copyOf(document);
--- 817,830 ----
  			document.setMaxDepth(document.getMaxDepth() + 1);
  			document.calculateTreeLevelStartPoints();

+ 			// new document is valid: remove old document
  			if (oldDoc != null) {
  				LOG.debug("removing old document " + oldDoc.getFileName());
  				if (oldDoc.getResourceType() == DocumentImpl.BINARY_FILE)
  					broker.removeBinaryResource((BinaryDocument) oldDoc);
! 				else {
! //					broker.checkTree(oldDoc);
  					broker.removeDocument(getName() + '/' + oldDoc.getFileName(), false);
+ 				}
  				// we continue to use the old document object and just replace its contents
  				oldDoc.copyOf(document);
***************
*** 826,831 ****
  		    if(oldDoc != null) oldDoc.getUpdateLock().release(Lock.WRITE_LOCK);
  		    throw e;
- 		} finally {
- 			lock.release();
  		}
  		return document;
--- 850,853 ----
***************
*** 919,925 ****
  	 */
  	private void checkPermissions(DBBroker broker, String name, DocumentImpl oldDoc) throws LockException, PermissionDeniedException {
! 		lock.acquire(Lock.WRITE_LOCK);
! 		if (hasDocument(name) && (oldDoc ) != null) {

  			// check if the document is locked by another user
  			User lockUser = oldDoc.getUserLock();
--- 941,947 ----
  	 */
  	private void checkPermissions(DBBroker broker, String name, DocumentImpl oldDoc) throws LockException, PermissionDeniedException {
! 		if (oldDoc != null) {

+ 			LOG.debug("Found old doc " + oldDoc.getDocId() + "; identity = " + oldDoc.hashCode());
  			// check if the document is locked by another user
  			User lockUser = oldDoc.getUserLock();
***************
*** 929,934 ****

  			// check if the document is currently being changed by someone else
! 			Lock oldLock = oldDoc.getUpdateLock();
! 			oldLock.acquire(Lock.WRITE_LOCK);

  			// do we have permissions for update?
--- 951,955 ----

  			// check if the document is currently being changed by someone else
! 			oldDoc.getUpdateLock().acquire(Lock.WRITE_LOCK);

  			// do we have permissions for update?
***************
*** 962,972 ****
  		DocumentImpl document = null, oldDoc = null;
  		XMLReader reader;
! 		oldDoc = getDocument(broker, name);
! 		document = new DocumentImpl(broker, name,	this);
! 		reader = getReader(broker);
! 
! 		// first pass: parse the document to determine tree structure
! 		document = determineTreeStructure(broker, name, document, oldDoc, reader, source);
! 
  		// reset the input source
  		try {
--- 983,998 ----
  		DocumentImpl document = null, oldDoc = null;
  		XMLReader reader;
! 		try {
! 			lock.acquire(Lock.WRITE_LOCK);
! 			oldDoc = getDocument(broker, name);
! 			document = new DocumentImpl(broker, name,	this);
! 			reader = getReader(broker);
! 	
! 			// first pass: parse the document to determine tree structure
! 			document = determineTreeStructure(broker, name, document, oldDoc, reader, source);
! 		} finally {
! 			lock.release();
! 		}
! 		
  		// reset the input source
  		try {
***************
*** 991,998 ****
  				throw new EXistException(e);
  			}
! 	
! 			if(oldDoc == null)
! 			    addDocument(broker, document);
! 			broker.addDocument(this, document);
  			broker.closeDocument();
  			broker.flush();
--- 1017,1027 ----
  				throw new EXistException(e);
  			}
! 
! 			if(oldDoc == null) {
! 				addDocument(broker, document);
! 				broker.addDocument(this, document);
! 			} else {
! 				broker.updateDocument(document);
! 			}
  			broker.closeDocument();
  			broker.flush();
***************
*** 1027,1034 ****
  		DocumentImpl document, oldDoc = null;
  		DOMStreamer streamer;
- 		oldDoc = getDocument(broker, name);
- 		document = new DocumentImpl(broker, name,	this);
- 		
  		try {
  			checkPermissions(broker, name, oldDoc);

--- 1056,1064 ----
  		DocumentImpl document, oldDoc = null;
  		DOMStreamer streamer;
  		try {
+ 			lock.acquire(Lock.WRITE_LOCK);
+ 			oldDoc = getDocument(broker, name);
+ 			document = new DocumentImpl(broker, name,	this);
+ 		
  			checkPermissions(broker, name, oldDoc);

***************
*** 1093,1096 ****
--- 1123,1127 ----
  			lock.release();
  		}//ffffffffffffffffffffffffffffffffffffffffffffffff
+ 		
  		try {
  			// second pass: store the document
***************
*** 1099,1105 ****
  			streamer.serialize(node, true);

! 			if(oldDoc == null)
! 			    addDocument(broker, document);
! 			broker.addDocument(this, document);
  			broker.closeDocument();
  			broker.flush();
--- 1130,1139 ----
  			streamer.serialize(node, true);

! 			if(oldDoc == null) {
! 				addDocument(broker, document);
! 				broker.addDocument(this, document);
! 			} else {
! 				broker.updateDocument(document);
! 			}
  			broker.closeDocument();
  			broker.flush();

[Exist-commits] eXist-1.0/src/org/exist/collections Collection.java,1.41,1.42

eXist-db is a feature rich Open Source native XML database

[Exist-commits] eXist-1.0/src/org/exist/collections Collection.java,1.41,1.42