ConcurrentModificationException in I2PAppContext.getProperties

[zab]

As reported by user1@kytv : ​http://pastethis.i2p/show/1585/

<sermon>System properties should not be modified during runtime.</sermon>

Quick fix:

#
# old_revision [d4d2cdadeb5c65143ca9c96933078a305a3be128]
#
# patch "core/java/src/net/i2p/I2PAppContext.java"
#  from [3895cfcc735e60cd9c3d0ecd144e36f42d7869b4]
#    to [679cba87e6f0e721d8107aa58a0ed9e38a9414af]
#
============================================================
--- core/java/src/net/i2p/I2PAppContext.java	3895cfcc735e60cd9c3d0ecd144e36f42d7869b4
+++ core/java/src/net/i2p/I2PAppContext.java	679cba87e6f0e721d8107aa58a0ed9e38a9414af
@@ -516,9 +516,16 @@ public class I2PAppContext {
      * @return set of Strings containing the names of defined system properties
      */
     public Set getPropertyNames() { 
-        Set names = new HashSet(System.getProperties().keySet());
-        if (_overrideProps != null)
-            names.addAll(_overrideProps.keySet());
+    	Properties system = System.getProperties();
+    	Set names;
+        synchronized(system) {
+        	names = new HashSet(System.getProperties().keySet());
+        }
+        if (_overrideProps != null) {
+        	synchronized(_overrideProps) {
+        		names.addAll(_overrideProps.keySet());
+        	}
+        }
         return names;
     }
     
@@ -532,8 +539,13 @@ public class I2PAppContext {
      */
     public Properties getProperties() { 
         Properties rv = new Properties();
-        rv.putAll(System.getProperties());
-        rv.putAll(_overrideProps);
+        Properties system = System.getProperties();
+        synchronized(system) {
+        	rv.putAll(system);
+        }
+        synchronized(_overrideProps){
+        	rv.putAll(_overrideProps);
+        }
         return rv;
     }
     

[zzz]

Truly one in a zillion, never reported before.

I don't think that's a legit fix above as that assumes a certain Properties implementation.

I found 3 ideas in a quick web search:
1) clone()
2) keySet/EntrySet toArray(), then iterate thru the array
3) catch CME and loop until success

1) seems the most sensible to me. There's a few other places in the code that have to be fixed too.

re: sermon, never heard that before, but in most cases for us it's a symptom of lazy programming, just a static place to hang something. The update redesign currently in the i2p.i2p.zzz.update branch will fix a few of them, but routerconsole will still be a huge offender.

[zzz]

Fixed I2PAppContext, streaming, SAM, and I2CP using clone() in 0.9.1-4.