Skip to content

Commit

Permalink
Resolves #268: Calling a value is not thread safe (return another value)
Browse files Browse the repository at this point in the history
The bug was introduced by commit 1acb314 which was supposed to resolve
a thread contention issue #254, which now I think, it was not an issue
since it was required to have some synchronization mechanism to avoid
problems like the one happened here.
  • Loading branch information
Luigi R. Viggiano committed Jun 6, 2020
1 parent 00096df commit 1f89055
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 15 deletions.
23 changes: 8 additions & 15 deletions owner/src/main/java/org/aeonbits/owner/Converters.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;

import static java.beans.PropertyEditorManager.findEditor;
import static java.lang.Boolean.getBoolean;
import static java.lang.reflect.Modifier.isStatic;
import static org.aeonbits.owner.Converters.SpecialValue.NULL;
import static org.aeonbits.owner.Converters.SpecialValue.SKIP;
Expand Down Expand Up @@ -151,16 +153,6 @@ Object tryConvert(Method targetMethod, Class<?> targetType, String text) {
throw unsupportedConversion(e, targetType, text);
}
}

private PropertyEditor findEditor(Class<?> targetType) {
PropertyEditor editor = PROPERTY_BY_CLASS.get(targetType);
if (editor == null) {
editor = PropertyEditorManager.findEditor(targetType);
if (editor != null)
PROPERTY_BY_CLASS.put(targetType, editor);
}
return editor;
}

private boolean canUsePropertyEditors() {
return isPropertyEditorAvailable && !isPropertyEditorDisabled;
Expand Down Expand Up @@ -266,13 +258,14 @@ private static Object convertWithConverterClass(Method targetMethod, String text
return result;
}

private static final Map<Class<?>, Class<? extends Converter<?>>> converterRegistry = new ConcurrentHashMap<Class<?>, Class<? extends Converter<?>>>();
private static final Map<Class<?>, Class<? extends Converter<?>>> converterRegistry =
new ConcurrentHashMap<Class<?>, Class<? extends Converter<?>>>();

private static final boolean isPropertyEditorAvailable = isClassAvailable("java.beans.PropertyEditorManager");

private static final boolean isPropertyEditorDisabled = Boolean.getBoolean("org.aeonbits.owner.property.editor.disabled");
private static final boolean isPropertyEditorAvailable =
isClassAvailable("java.beans.PropertyEditorManager");

private static final Map<Class<?>, PropertyEditor> PROPERTY_BY_CLASS = new WeakHashMap<Class<?>, PropertyEditor>();
private static final boolean isPropertyEditorDisabled =
getBoolean("org.aeonbits.owner.property.editor.disabled");

abstract Object tryConvert(Method targetMethod, Class<?> targetType, String text);

Expand Down
90 changes: 90 additions & 0 deletions owner/src/test/java/org/aeonbits/owner/issues/Issue268.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package org.aeonbits.owner.issues;

import org.aeonbits.owner.Config;
import org.aeonbits.owner.ConfigFactory;
import org.junit.Test;

import static java.lang.Thread.sleep;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertFalse;

// Issue #268
public class Issue268 {
interface MyConfig extends Config {
@DefaultValue("Pasha")
String firstName();
@DefaultValue("Bairov")
String lastName();
}

@Test
public void testConcurrentAccess() throws InterruptedException {
final MyConfig cfg = ConfigFactory.create(MyConfig.class);

final Object semaphore = new Object();

final boolean[] exit = {false};
final boolean[] nameMismatch = { false };
final boolean[] lastNameMismatch = { false };
final boolean[] interrupted = {false, false};

Thread t1 = new Thread() {
@Override
public void run() {
try {
synchronized (semaphore) {
semaphore.wait();
}
for (int i = 0; i < 1000 && ! exit[0]; i++) {
String name = cfg.firstName();

if (!name.equals("Pasha")) {
System.out.println("name " + name);
nameMismatch[0] = true;
exit[0] = true;
}
}
} catch (InterruptedException e) {
interrupted[0] = true;
throw new IllegalStateException();
}
}
};

Thread t2 = new Thread() {
@Override
public void run() {
try {
synchronized (semaphore) {
semaphore.wait();
}
for (int i = 0; i < 1000 &&! exit[0] ; i++) {
String lastName = cfg.lastName();
if (!lastName.equals("Bairov")) {
System.out.println("lastName " + lastName);
lastNameMismatch[0] = true;
exit[0] = true;
}
}
} catch (InterruptedException e) {
interrupted[1] = true;
throw new IllegalStateException();
}
}
};
t1.start();
t2.start();

sleep(300);

synchronized (semaphore) {
semaphore.notifyAll();
}

t1.join();
t2.join();
assertFalse("mismatch on name", nameMismatch[0]);
assertFalse("mismatch on lastName", lastNameMismatch[0]);
assertArrayEquals(new boolean[] {false, false}, interrupted);
}
}

0 comments on commit 1f89055

Please sign in to comment.