* SusiMail:

- Queue deletions for a delayed background thread
   - Synch all folder access
   - NPE fixes
   - Javadoc fixes

apps/susimail/src/src/i2p/susi/util/Folder.java

@@ -42,7 +42,7 @@ import java.util.List;
  * and then fetch the content of the current page with
  * currentPageIterator().
  * 
- * Warning - unsynchronized - not thread safe
+ * All public methods are synchronized.
  * 
  * @author susi
  */
@@ -77,7 +77,7 @@ public class Folder<O extends Object> {
 	 * 
 	 * @return Returns the current page.
 	 */
-	public int getCurrentPage() {
+	public synchronized int getCurrentPage() {
 		return currentPage;
 	}
 
@@ -86,7 +86,7 @@ public class Folder<O extends Object> {
 	 * 
 	 * @param currentPage The current page to set.
 	 */
-	public void setCurrentPage(int currentPage) {
+	public synchronized void setCurrentPage(int currentPage) {
 		if( currentPage >= 1 && currentPage <= pages )
 			this.currentPage = currentPage;
 	}
@@ -96,7 +96,7 @@ public class Folder<O extends Object> {
 	 * 
 	 * @return Returns the size of the folder.
 	 */
-	public int getSize() {
+	public synchronized int getSize() {
 		return elements != null ? elements.length : 0;
 	}
 
@@ -104,7 +104,7 @@ public class Folder<O extends Object> {
 	 * Returns the number of pages in the folder.
 	 * @return Returns the number of pages.
 	 */
-	public int getPages() {
+	public synchronized int getPages() {
 		return pages;
 	}
 
@@ -114,7 +114,7 @@ public class Folder<O extends Object> {
 	 * 
 	 * @return Returns the pageSize.
 	 */
-	public int getPageSize() {
+	public synchronized int getPageSize() {
 		return pageSize > 0 ? pageSize : Config.getProperty( PAGESIZE, DEFAULT_PAGESIZE );
 	}
 
@@ -123,7 +123,7 @@ public class Folder<O extends Object> {
 	 * 
 	 * @param pageSize The page size to set.
 	 */
-	public void setPageSize(int pageSize) {
+	public synchronized void setPageSize(int pageSize) {
 		if( pageSize > 0 )
 			this.pageSize = pageSize;
 		update();
@@ -180,7 +180,7 @@ public class Folder<O extends Object> {
 	 * 
 	 * @param elements Array of Os.
 	 */
-	public void setElements( O[] elements )
+	public synchronized void setElements( O[] elements )
 	{
 		if (elements.length > 0) {
 			this.unsortedElements = elements;
@@ -198,17 +198,17 @@ public class Folder<O extends Object> {
 	 * 
 	 * @param element to remove
 	 */
-	public void removeElement(O element) {
+	public synchronized void removeElement(O element) {
 		removeElements(Collections.singleton(element));
 	}
 	
 	/**
 	 * Remove elements
 	 * 
-	 * @param elements to remove
+	 * @param elems to remove
 	 */
 	@SuppressWarnings("unchecked")
-	public void removeElements(Collection<O> elems) {
+	public synchronized void removeElements(Collection<O> elems) {
 		if (elements != null) {
 			List<O> list = new ArrayList<O>(Arrays.asList(elements));
 			for (O e : elems) {
@@ -228,9 +228,14 @@ public class Folder<O extends Object> {
 	
 	/**
 	 * Returns an iterator containing the elements on the current page.
+         * This iterator is over a copy of the current page, and so
+         * is thread safe w.r.t. other operations on this folder,
+         * but will not reflect subsequent changes, and iter.remove()
+         * will not change the folder.
+         *
 	 * @return Iterator containing the elements on the current page.
 	 */
-	public Iterator<O> currentPageIterator()
+	public synchronized Iterator<O> currentPageIterator()
 	{
 		ArrayList<O> list = new ArrayList<O>();
 		if( elements != null ) {
@@ -247,7 +252,7 @@ public class Folder<O extends Object> {
 	/**
 	 * Turns folder to next page.
 	 */
-	public void nextPage()
+	public synchronized void nextPage()
 	{
 		currentPage++;
 		if( currentPage > pages )
@@ -257,7 +262,7 @@ public class Folder<O extends Object> {
 	/**
 	 * Turns folder to previous page.
 	 */
-	public void previousPage()
+	public synchronized void previousPage()
 	{
 		currentPage--;
 		if( currentPage < 1 )
@@ -267,7 +272,7 @@ public class Folder<O extends Object> {
 	/**
 	 * Sets folder to display first page.
 	 */
-	public void firstPage()
+	public synchronized void firstPage()
 	{
 		currentPage = 1;
 	}
@@ -275,7 +280,7 @@ public class Folder<O extends Object> {
 	/**
 	 * Sets folder to display last page.
 	 */
-	public void lastPage()
+	public synchronized void lastPage()
 	{
 		currentPage = pages;
 	}
@@ -287,7 +292,7 @@ public class Folder<O extends Object> {
 	 * @param id ID to identify the Comparator with @link sortBy()
 	 * @param sorter a Comparator to sort the Array given by @link setElements()
 	 */
-	public void addSorter( String id, Comparator<O> sorter )
+	public synchronized void addSorter( String id, Comparator<O> sorter )
 	{
 		this.sorter.put( id, sorter );
 	}
@@ -299,7 +304,7 @@ public class Folder<O extends Object> {
 	 * 
 	 * @param id ID to identify the Comparator stored with @link addSorter()
 	 */
-	public void sortBy( String id )
+	public synchronized void sortBy( String id )
 	{
 		currentSorter = sorter.get( id );
 		if (sortingDirection == SortOrder.UP)
@@ -313,7 +318,7 @@ public class Folder<O extends Object> {
 	 * @param x Position of the element on the current page.
 	 * @return Element on the current page on the given position.
 	 */
-	public O getElementAtPosXonCurrentPage( int x )
+	public synchronized O getElementAtPosXonCurrentPage( int x )
 	{
 		O result = null;
 		if( elements != null ) {
@@ -332,7 +337,7 @@ public class Folder<O extends Object> {
 	 * 
 	 * @param direction @link UP or @link DOWN
 	 */
-	public void setSortingDirection(SortOrder direction)
+	public synchronized void setSortingDirection(SortOrder direction)
 	{
 		sortingDirection = direction;
 	}
@@ -342,7 +347,7 @@ public class Folder<O extends Object> {
 	 * 
 	 * @return First element.
 	 */
-	public O getFirstElement()
+	public synchronized O getFirstElement()
 	{
 		return elements == null ? null : getElement( 0 );
 	}
@@ -352,7 +357,7 @@ public class Folder<O extends Object> {
 	 * 
 	 * @return Last element.
 	 */
-	public O getLastElement()
+	public synchronized O getLastElement()
 	{
 		return elements == null ? null : getElement(  elements.length - 1 );
 	}
@@ -379,7 +384,7 @@ public class Folder<O extends Object> {
 	 * @param element
 	 * @return The next element
 	 */
-	public O getNextElement( O element )
+	public synchronized O getNextElement( O element )
 	{
 		O result = null;
 		
@@ -399,7 +404,7 @@ public class Folder<O extends Object> {
 	 * @param element
 	 * @return The previous element
 	 */
-	public O getPreviousElement( O element )
+	public synchronized O getPreviousElement( O element )
 	{
 		O result = null;
 		
@@ -431,7 +436,7 @@ public class Folder<O extends Object> {
 	/**
 	 * Returns true, if folder shows points to the last page.
 	 */
-	public boolean isLastPage()
+	public synchronized boolean isLastPage()
 	{
 		return currentPage == pages;
 	}
@@ -439,7 +444,7 @@ public class Folder<O extends Object> {
 	/**
 	 * Returns true, if folder shows points to the first page.
 	 */
-	public boolean isFirstPage()
+	public synchronized boolean isFirstPage()
 	{
 		return currentPage == 1;
 	}
@@ -449,7 +454,7 @@ public class Folder<O extends Object> {
 	 * 
 	 * @param element
 	 */
-	public boolean isLastElement( O element )
+	public synchronized boolean isLastElement( O element )
 	{
 		if( elements == null )
 			return false;
@@ -461,7 +466,7 @@ public class Folder<O extends Object> {
 	 * 
 	 * @param element
 	 */
-	public boolean isFirstElement( O element )
+	public synchronized boolean isFirstElement( O element )
 	{
 		if( elements == null )
 			return false;

apps/susimail/src/src/i2p/susi/webmail/Mail.java

@@ -223,7 +223,6 @@ class Mail {
 	 * Adds all items from the list
 	 * to the builder, separated by tabs.
 	 * 
-	 * @param text comma-separated
 	 * @param buf out param
 	 * @param prefix prepended to the addresses
 	 */

apps/susimail/src/src/i2p/susi/webmail/MailCache.java

@@ -261,7 +261,7 @@ class MailCache {
 		}
 		if (toDelete.isEmpty())
 			return;
-		mailbox.delete(toDelete);
+		mailbox.queueForDeletion(toDelete);
 	}
 
 	/**

apps/susimail/src/src/i2p/susi/webmail/WebMail.java

@@ -886,7 +886,7 @@ public class WebMail extends HttpServlet
 					/*
 					 * extract original sender from Reply-To: or From:
 					 */
-					MailPart part = mail.getPart();
+					MailPart part = mail != null ? mail.getPart() : null;
 					if (part != null) {
 						if( reply || replyAll ) {
 							if( mail.reply != null && Mail.validateAddress( mail.reply ) )
@@ -1173,7 +1173,7 @@ public class WebMail extends HttpServlet
 			try {
 				int hashCode = Integer.parseInt( str );
 				Mail mail = sessionObject.mailCache.getMail( sessionObject.showUIDL, MailCache.FETCH_ALL );
-				MailPart part = getMailPartFromHashCode( mail.getPart(), hashCode );
+				MailPart part = mail != null ? getMailPartFromHashCode( mail.getPart(), hashCode ) : null;
 				if( part != null )
 					sessionObject.showAttachment = part;
 			}
@@ -1910,6 +1910,8 @@ public class WebMail extends HttpServlet
 		for( Iterator<String> it = sessionObject.folder.currentPageIterator(); it != null && it.hasNext(); ) {
 			String uidl = it.next();
 			Mail mail = sessionObject.mailCache.getMail( uidl, MailCache.FETCH_HEADER );
+			if (mail == null)
+				continue;
 			String link = "<a href=\"" + myself + "?" + SHOW + "=" + i + "\">";
 			
 			boolean idChecked = false;

apps/susimail/src/src/i2p/susi/webmail/pop3/DelayedDeleter.java

@@ -0,0 +1,100 @@
+package i2p.susi.webmail.pop3;
+
+import i2p.susi.debug.Debug;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Set;
+
+import net.i2p.I2PAppContext;
+import net.i2p.util.I2PAppThread;
+import net.i2p.util.ConcurrentHashSet;
+import net.i2p.util.SimpleTimer2;
+
+
+/**
+ *  Queue UIDLs for later deletion
+ *
+ *  @since 0.9.13
+ */
+class DelayedDeleter {
+
+	private final POP3MailBox mailbox;
+	private final Set<String> toDelete;
+	private final SimpleTimer2.TimedEvent timer;
+	private volatile boolean isDeleting;
+	private volatile boolean isDead;
+
+	private final long CHECK_TIME = 5*60*1000;
+	private final long MIN_IDLE = 5*60*1000;
+
+	public DelayedDeleter(POP3MailBox mailbox) {
+		this.mailbox = mailbox;
+		toDelete = new ConcurrentHashSet<String>();
+		timer = new Checker();
+	}
+
+	public void queueDelete(String uidl) {
+		toDelete.add(uidl);
+	}
+
+	public void removeQueued(String uidl) {
+		toDelete.remove(uidl);
+	}
+
+	public Collection<String> getQueued() {
+		List<String> rv = new ArrayList<String>(toDelete);
+		return rv;
+	}
+
+	public void cancel() {
+		isDead = true;
+		timer.cancel();
+	}
+
+	private class Checker extends SimpleTimer2.TimedEvent {
+
+		public Checker() {
+			super(I2PAppContext.getGlobalContext().simpleTimer2(), CHECK_TIME + 5*1000);
+		}
+
+	        public void timeReached() {
+			if (isDead)
+				return;
+			if (!toDelete.isEmpty() && !isDeleting) {
+				long idle = System.currentTimeMillis() - mailbox.getLastActivity();
+				if (idle >= MIN_IDLE) {
+					Debug.debug(Debug.DEBUG, "Threading delayed delete for " + toDelete.size() +
+							" mails after " + idle + " ms idle");
+					Thread t = new Deleter();
+					isDeleting = true;
+					t.start();
+				} else {
+					Debug.debug(Debug.DEBUG, "Nothing to delete");
+				}
+			}
+			schedule(CHECK_TIME);
+		}
+	}
+
+	private class Deleter extends I2PAppThread {
+
+		public Deleter() {
+			super("Susimail-Delete");
+		}
+
+	        public void run() {
+			try {
+				List<String> uidls = new ArrayList<String>(toDelete);
+				Collection<String> deleted = mailbox.delete(uidls);
+				Debug.debug(Debug.DEBUG, "Deleted " + deleted.size() + " of " + toDelete.size() + " mails");
+				toDelete.removeAll(deleted);
+			} finally {		
+				isDeleting = false;
+				if (!isDead)
+					timer.schedule(CHECK_TIME);
+			}		
+		}
+	}
+}

apps/susimail/src/src/i2p/susi/webmail/pop3/POP3MailBox.java

@@ -36,6 +36,8 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicLong;
 
 import net.i2p.data.DataHelper;
 
@@ -63,8 +65,10 @@ public class POP3MailBox {
 	private final HashMap<String, Integer> uidlToID;
 
 	private Socket socket;
+	private final AtomicLong lastActive;
 
 	private final Object synchronizer;
+	private final DelayedDeleter delayedDeleter;
 
 	/**
 	 * Does not connect. Caller must call connectToServer() if desired.
@@ -87,6 +91,8 @@ public class POP3MailBox {
 		synchronizer = new Object();
 		// this appears in the UI so translate
 		lastLine = _("No response from server");
+		lastActive = new AtomicLong(System.currentTimeMillis());
+		delayedDeleter = new DelayedDeleter(this);
 	}
 
 	/**
@@ -232,10 +238,12 @@ public class POP3MailBox {
 
 	/**
 	 * Call performDelete() after this or they will come back
+	 * UNUSED
 	 * 
 	 * @param uidl
 	 * @return Success of delete operation: true if successful.
 	 */
+/****
 	public boolean delete( String uidl )
 	{
 		Debug.debug(Debug.DEBUG, "delete(" + uidl + ")");
@@ -253,15 +261,28 @@ public class POP3MailBox {
 			return delete(id);
 		}
 	}
+****/
 
 	/**
-	 * Delete all at once, close and reconnect
+	 * Queue for later deletion. Non-blocking.
-	 * Do NOT call performDelete() after this
-	 * Does not provide any success/failure result
 	 * 
 	 * @since 0.9.13
 	 */
-	public void delete(Collection<String> uidls) {
+	public void queueForDeletion(Collection<String> uidls) {
+		for (String uidl : uidls) {
+			delayedDeleter.queueDelete(uidl);
+		}
+	}
+
+	/**
+	 * Delete all at once and close. Does not reconnect.
+	 * Do NOT call performDelete() after this.
+	 * Returns all UIDLs successfully deleted OR were not known by the server.
+	 * 
+	 * @since 0.9.13
+	 */
+	Collection<String> delete(Collection<String> uidls) {
+		List<String> rv = new ArrayList<String>(uidls.size());
 		List<SendRecv> srs = new ArrayList<SendRecv>(uidls.size() + 1);
 		synchronized( synchronizer ) {
 			try {
@@ -269,26 +290,41 @@ public class POP3MailBox {
 				checkConnection();
 			} catch (IOException ioe) {
 				Debug.debug( Debug.DEBUG, "Error deleting: " + ioe);
-				return;
+				return rv;
 			}
 			for (String uidl : uidls) {
 				int id = getIDfromUIDL(uidl);
-				if (id < 0)
+				if (id < 0) {
+					// presumed already deleted
+					rv.add(uidl);
 					continue;
+				}
 				SendRecv sr = new SendRecv("DELE " + id, Mode.A1);
+				sr.savedObject = uidl;
 				srs.add(sr);
 			}
 			if (srs.isEmpty())
-				return;
+				return rv;
 			// TODO don't quit now, just set timer to quit later
-			SendRecv sr = new SendRecv("QUIT", Mode.A1);
+			SendRecv quit = new SendRecv("QUIT", Mode.A1);
-			srs.add(sr);
+			srs.add(quit);
 			try {
 				sendCmds(srs);
+				// do NOT call close() here, we included QUIT above
 				try {
 					socket.close();
 				} catch (IOException e) {}
 				clear();
+				// result of QUIT
+				boolean success = srs.get(srs.size() - 1).result;
+				if (success) {
+					for (int i = 0; i < srs.size() - 1; i++) {
+						SendRecv sr = srs.get(i);
+						// ignore sr.result, if it failed it's because
+						// it's already deleted
+						rv.add((String) sr.savedObject);
+					}
+				}
 				// why reconnect?
 				//connect();
 			} catch (IOException ioe) {
@@ -296,14 +332,17 @@ public class POP3MailBox {
 				// todo maybe
 			}
 		}
+		return rv;
 	}
 	
 	/**
 	 * delete message on pop3 server
+	 * UNUSED
 	 * 
 	 * @param id message id
 	 * @return Success of delete operation: true if successful.
 	 */
+/****
 	private boolean delete(int id)
 	{
 		Debug.debug(Debug.DEBUG, "delete(" + id + ")");
@@ -320,6 +359,7 @@ public class POP3MailBox {
 		}
 		return result;
 	}
+****/
 
 	/**
 	 * Get cached size of a message (via previous LIST command).
@@ -387,6 +427,24 @@ public class POP3MailBox {
 		}
 	}
 
+	/**
+	 * Timestamp.
+	 * 
+	 * @since 0.9.13
+	 */
+	private void updateActivity() {
+		lastActive.set(System.currentTimeMillis());
+	}
+
+	/**
+	 * Timestamp.
+	 * 
+	 * @since 0.9.13
+	 */
+	long getLastActivity() {
+		return lastActive.get();
+	}
+
 	/**
 	 * 
 	 * @param response line starting with +OK
@@ -628,6 +686,7 @@ public class POP3MailBox {
 		sendCmd1aNoWait(cmd);
 		socket.getOutputStream().flush();
 		String foo = DataHelper.readLine(socket.getInputStream());
+		updateActivity();
 		// Debug.debug(Debug.DEBUG, "sendCmd1a: read " + read + " bytes");
 		if (foo != null) {
 			lastLine = foo;
@@ -684,6 +743,7 @@ public class POP3MailBox {
 				}
 			}
 			String foo = DataHelper.readLine(in);
+			updateActivity();
 			if (foo == null) {
 				lastError = _("No response from server");
 				throw new IOException(lastError);
@@ -747,6 +807,7 @@ public class POP3MailBox {
 		Debug.debug(Debug.DEBUG, "sendCmd1a(" + msg + ")");
 		cmd += "\r\n";
 		socket.getOutputStream().write(DataHelper.getASCII(cmd));
+		updateActivity();
 	}
 
 	/**
@@ -829,6 +890,7 @@ public class POP3MailBox {
 		StringBuilder buf = new StringBuilder(512);
 		ByteArrayOutputStream baos = new ByteArrayOutputStream(4096);
 		while (DataHelper.readLine(input, buf)) {
+			updateActivity();
 			int len = buf.length();
 			if (len == 0)
 				break; // huh? no \r?
@@ -867,6 +929,7 @@ public class POP3MailBox {
 		long startTime = System.currentTimeMillis();
 		StringBuilder buf = new StringBuilder(512);
 		while (DataHelper.readLine(input, buf)) {
+			updateActivity();
 			int len = buf.length();
 			if (len == 0)
 				break; // huh? no \r?
@@ -918,14 +981,25 @@ public class POP3MailBox {
 	}
 
 	/**
-	 *  Close without waiting for response
+	 *  Close without waiting for response,
+	 *  and remove any delayed tasks and resources.
+	 */
+	public void destroy() {
+		delayedDeleter.cancel();
+		close(false);
+	}
+
+	/**
+	 *  Close without waiting for response.
+	 *  Deletes all queued deletions.
 	 */
 	public void close() {
 		close(false);
 	}
 
 	/**
-	 *  Close and optionally waiting for response
+	 *  Close and optionally wait for response.
+	 *  Deletes all queued deletions.
 	 *  @since 0.9.13
 	 */
 	private void close(boolean shouldWait) {
@@ -933,10 +1007,34 @@ public class POP3MailBox {
 			Debug.debug(Debug.DEBUG, "close()");
 			if (socket != null && socket.isConnected()) {
 				try {
-					if (shouldWait)
+					Collection<String> toDelete = delayedDeleter.getQueued();
-						sendCmd1a("QUIT");
+					Map<String, Integer> sendDelete = new HashMap<String, Integer>(toDelete.size());
-					else
+					for (String uidl : toDelete) {
+						int id = getIDfromUIDL(uidl);
+						if (id >= 0) {
+							sendDelete.put(uidl, Integer.valueOf(id));
+						}
+					}
+					if (shouldWait) {
+						if (!sendDelete.isEmpty()) {
+							// Verify deleted, remove from the delete queue
+							// this does the quit and close
+							Collection<String> deleted = delete(sendDelete.keySet());
+							for (String uidl : deleted) {
+								delayedDeleter.removeQueued(uidl);
+							}
+						} else {
+							sendCmd1a("QUIT");
+						}
+					} else {
+						if (!sendDelete.isEmpty()) {
+							// spray and pray the deletions, don't remove from delete queue
+							for (Integer id : sendDelete.values()) {
+								sendCmd1aNoWait("DELE " + id);
+							}
+						}
 						sendCmd1aNoWait("QUIT");
+					}
 					socket.close();
 				} catch (IOException e) {}
 			}
@@ -1012,7 +1110,9 @@ public class POP3MailBox {
 
 	/**
 	 *  Close and reconnect. Takes a while.
+	 *  UNUSED
 	 */
+/****
 	public void performDelete()
 	{
 		synchronized( synchronizer ) {
@@ -1021,7 +1121,9 @@ public class POP3MailBox {
 			//connect();
 		}
 	}
+****/
 
+	/** for SendRecv */
 	private enum Mode {
 		/** no extra lines (sendCmd1a) */
 		A1,

history.txt

@@ -1,3 +1,8 @@
+2014-04-23 zzz
+ * SusiMail:
+   - Queue deletions for a later thread
+   - Synch all folder access
+
 2014-04-22 zzz
  * SusiMail:
    - Add persistent cache

router/java/src/net/i2p/router/RouterVersion.java

@@ -18,7 +18,7 @@ public class RouterVersion {
     /** deprecated */
     public final static String ID = "Monotone";
     public final static String VERSION = CoreVersion.VERSION;
-    public final static long BUILD = 8;
+    public final static long BUILD = 9;
 
     /** for example "-test" */
     public final static String EXTRA = "";