Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ConcurrentModificationException on creating Config #203

Closed
toboro opened this issue May 16, 2017 · 5 comments
Closed

ConcurrentModificationException on creating Config #203

toboro opened this issue May 16, 2017 · 5 comments

Comments

@toboro
Copy link

@toboro toboro commented May 16, 2017

Hi,

first of all, this is a great api. We use it for about half a year and it is very handy :-) great work!

Now we get an Exception in a multithreaded environment. I analysed it, and found out, that it occurs when the system properties are modified, while a owner Config object is created:

Caused by: java.util.ConcurrentModificationException
	at java.util.Hashtable$Enumerator.next(Hashtable.java:1167)
	at java.util.Hashtable.putAll(Hashtable.java:586)
	at org.aeonbits.owner.VariablesExpander.<init>(VariablesExpander.java:31)
	at org.aeonbits.owner.DefaultFactory.create(DefaultFactory.java:42)
	at org.aeonbits.owner.ConfigFactory.create(ConfigFactory.java:66)

I wrote a junit testcase to reproduce it. And I also found a Workaround for it, by use another SystemProvider (from your api) which take a copy from the system properties made with an enumeration, because enumerations don't throw ConcurrentModificationExceptions.

Here is the testcase:

package org.aeonbits.owner;

import java.util.*;
import java.util.concurrent.*;
import java.util.concurrent.atomic.AtomicBoolean;
import org.aeonbits.owner.Util.SystemProvider;
import org.junit.Test;

public class OwnerTest {

  public static interface SomeConfig extends Config {
    //
  }

  @Test
  public void testSystemProperties() throws Exception {

    //uncomment the next line to get the test working...
//    setAlternateSystemProvider();

    ExecutorService exe = Executors.newFixedThreadPool(2);
    try {
      final AtomicBoolean running = new AtomicBoolean(true);

      Future<Void> configLoader = exe.submit(new Callable<Void>() {
        @Override
        public Void call() throws Exception {
          //on most tries it fails on the first iteration.
          for(int cnt = 0; cnt < 100; cnt++) {
            ConfigFactory.create(SomeConfig.class);
          }
          return null;
        }
      });

      Future<Void> sysPropModifier = exe.submit(new Callable<Void>() {
        @Override
        public Void call() throws Exception {
          while (running.get()) {
            System.setProperty("Foo", "Bar");
            System.getProperties().remove("Foo");
          }
          return null;
        }
      });

      configLoader.get();
      running.set(false);
      sysPropModifier.get();
    }
    finally {
      exe.shutdown();
    }
  }

  private static void setAlternateSystemProvider() {
    Util.system = new SystemProvider() {
      @Override
      public String getProperty(String key) {
        return System.getProperty(key);
      }

      @Override
      public Map<String, String> getenv() {
        return System.getenv();
      }

      @Override
      public Properties getProperties() {
        Properties res = new Properties();
        Enumeration<Object> keys = System.getProperties().keys();
        while(keys.hasMoreElements()) {
          Object key = keys.nextElement();
          Object value = System.getProperties().get(key);
          if(value != null) {
            res.put(key, value);
          }
        }
        return res;
      }
    };
  }
}
@lviggiano
Copy link
Collaborator

@lviggiano lviggiano commented May 16, 2017

@richmidwinter
Copy link

@richmidwinter richmidwinter commented Jun 17, 2017

Might be better off replacing the contents of that getProperties() method with a clone() call:

public Properties getProperties() { return (Properties) System.getProperties().clone(); }

@lviggiano
Copy link
Collaborator

@lviggiano lviggiano commented Jun 29, 2017

👍

@hoterran
Copy link

@hoterran hoterran commented Sep 21, 2017

@lviggiano this is confused bug for a long time。 can release new version after fix it ?

Litemn pushed a commit to Litemn/owner that referenced this issue Sep 26, 2018
lviggiano added a commit that referenced this issue Feb 5, 2019
@lviggiano
Copy link
Collaborator

@lviggiano lviggiano commented Feb 13, 2019

resolved by pull request #241
will be released asap

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants