diff --git a/apps/susimail/src/src/i2p/susi/util/Folder.java b/apps/susimail/src/src/i2p/susi/util/Folder.java index afd04c820a..75ae2088bd 100644 --- a/apps/susimail/src/src/i2p/susi/util/Folder.java +++ b/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 { * * @return Returns the current page. */ - public int getCurrentPage() { + public synchronized int getCurrentPage() { return currentPage; } @@ -86,7 +86,7 @@ public class Folder { * * @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 { * * @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 { * 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 { * * @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 { * * @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 { * * @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 { * * @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 elems) { + public synchronized void removeElements(Collection elems) { if (elements != null) { List list = new ArrayList(Arrays.asList(elements)); for (O e : elems) { @@ -228,9 +228,14 @@ public class Folder { /** * 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 currentPageIterator() + public synchronized Iterator currentPageIterator() { ArrayList list = new ArrayList(); if( elements != null ) { @@ -247,7 +252,7 @@ public class Folder { /** * Turns folder to next page. */ - public void nextPage() + public synchronized void nextPage() { currentPage++; if( currentPage > pages ) @@ -257,7 +262,7 @@ public class Folder { /** * Turns folder to previous page. */ - public void previousPage() + public synchronized void previousPage() { currentPage--; if( currentPage < 1 ) @@ -267,7 +272,7 @@ public class Folder { /** * Sets folder to display first page. */ - public void firstPage() + public synchronized void firstPage() { currentPage = 1; } @@ -275,7 +280,7 @@ public class Folder { /** * Sets folder to display last page. */ - public void lastPage() + public synchronized void lastPage() { currentPage = pages; } @@ -287,7 +292,7 @@ public class Folder { * @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 sorter ) + public synchronized void addSorter( String id, Comparator sorter ) { this.sorter.put( id, sorter ); } @@ -299,7 +304,7 @@ public class Folder { * * @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 { * @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 { * * @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 { * * @return First element. */ - public O getFirstElement() + public synchronized O getFirstElement() { return elements == null ? null : getElement( 0 ); } @@ -352,7 +357,7 @@ public class Folder { * * @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 { * @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 { * @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 { /** * 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 { /** * 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 { * * @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 { * * @param element */ - public boolean isFirstElement( O element ) + public synchronized boolean isFirstElement( O element ) { if( elements == null ) return false; diff --git a/apps/susimail/src/src/i2p/susi/webmail/Mail.java b/apps/susimail/src/src/i2p/susi/webmail/Mail.java index 9903589fb1..79bd287cb7 100644 --- a/apps/susimail/src/src/i2p/susi/webmail/Mail.java +++ b/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 */ diff --git a/apps/susimail/src/src/i2p/susi/webmail/MailCache.java b/apps/susimail/src/src/i2p/susi/webmail/MailCache.java index 8d0419932d..361db42ebe 100644 --- a/apps/susimail/src/src/i2p/susi/webmail/MailCache.java +++ b/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); } /** diff --git a/apps/susimail/src/src/i2p/susi/webmail/WebMail.java b/apps/susimail/src/src/i2p/susi/webmail/WebMail.java index 66b021927f..77f91d261d 100644 --- a/apps/susimail/src/src/i2p/susi/webmail/WebMail.java +++ b/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 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 = ""; boolean idChecked = false; diff --git a/apps/susimail/src/src/i2p/susi/webmail/pop3/DelayedDeleter.java b/apps/susimail/src/src/i2p/susi/webmail/pop3/DelayedDeleter.java new file mode 100644 index 0000000000..8ccdb91bc2 --- /dev/null +++ b/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 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(); + timer = new Checker(); + } + + public void queueDelete(String uidl) { + toDelete.add(uidl); + } + + public void removeQueued(String uidl) { + toDelete.remove(uidl); + } + + public Collection getQueued() { + List rv = new ArrayList(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 uidls = new ArrayList(toDelete); + Collection 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); + } + } + } +} diff --git a/apps/susimail/src/src/i2p/susi/webmail/pop3/POP3MailBox.java b/apps/susimail/src/src/i2p/susi/webmail/pop3/POP3MailBox.java index 0b918edda3..5db78c7a80 100644 --- a/apps/susimail/src/src/i2p/susi/webmail/pop3/POP3MailBox.java +++ b/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 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 - * Do NOT call performDelete() after this - * Does not provide any success/failure result + * Queue for later deletion. Non-blocking. * * @since 0.9.13 */ - public void delete(Collection uidls) { + public void queueForDeletion(Collection 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 delete(Collection uidls) { + List rv = new ArrayList(uidls.size()); List srs = new ArrayList(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); - srs.add(sr); + SendRecv quit = new SendRecv("QUIT", Mode.A1); + 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) - sendCmd1a("QUIT"); - else + Collection toDelete = delayedDeleter.getQueued(); + Map sendDelete = new HashMap(toDelete.size()); + 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 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, diff --git a/history.txt b/history.txt index fa37c838a6..d719d476ae 100644 --- a/history.txt +++ b/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 diff --git a/router/java/src/net/i2p/router/RouterVersion.java b/router/java/src/net/i2p/router/RouterVersion.java index a6204817e3..f2522cd4ed 100644 --- a/router/java/src/net/i2p/router/RouterVersion.java +++ b/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 = "";