Opened 9 years ago
Closed 9 years ago
#680 closed defect (fixed)
ConcurrentModificationException in I2PAppContext.getProperties
Reported by: | Zlatin Balevsky | Owned by: | zzz |
---|---|---|---|
Priority: | minor | Milestone: | 0.9.2 |
Component: | api/general | Version: | 0.9.1 |
Keywords: | Cc: | ||
Parent Tickets: | Sensitive: | no |
Description
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; }
Subtickets
Change History (2)
comment:1 Changed 9 years ago by
Milestone: | → 0.9.2 |
---|---|
Owner: | set to zzz |
Status: | new → accepted |
comment:2 Changed 9 years ago by
Component: | router/general → api/general |
---|---|
Resolution: | → fixed |
Status: | accepted → closed |
Fixed I2PAppContext, streaming, SAM, and I2CP using clone() in 0.9.1-4.
Note: See
TracTickets for help on using
tickets.
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.