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

PropertyEditor - Concurrency Issues #266

Closed
regbo opened this issue Apr 22, 2020 · 3 comments
Closed

PropertyEditor - Concurrency Issues #266

regbo opened this issue Apr 22, 2020 · 3 comments

Comments

@regbo
Copy link

regbo commented Apr 22, 2020

It looks like the PropertyEditor implementation in Converters is not thread safe. I believe the problem is in this code, and I think it should be synchronized.

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;

The code below illustrates the issue. ( "IndexServiceConfiguration" and "Configs" are custom but can be swapped out)

import java.util.Collections;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CountDownLatch;
import java.util.function.Consumer;

public class ConfigTest {

	public static void main(String[] args) throws InterruptedException {
		boolean propertyEditorDisabled = false;
		System.setProperty("org.aeonbits.owner.property.editor.disabled", Objects.toString(propertyEditorDisabled));
		IndexServiceConfiguration cfg = Configs.get(IndexServiceConfiguration.class);
		Set<String> uniqueTracker = Collections.newSetFromMap(new ConcurrentHashMap<String, Boolean>());
		Consumer<String> printMessage = msg -> {
			String out = String.format("msg:'%s' grant_type:%s uri:%s", msg, cfg.grant_type(), cfg.service_uri());
			if (uniqueTracker.add(out))
				System.out.println(out);
		};
		printMessage.accept("main thread (pre)");
		int testAmt = 1_000;
		CountDownLatch latch = new CountDownLatch(testAmt);
		for (int i = 0; i < testAmt; i++) {
			new Thread(() -> {
				printMessage.accept("in thread");
				latch.countDown();
			}).start();
		}
		latch.await();
		printMessage.accept("main thread (post)");
		System.err.println("end");
	}

}

When propertyEditorDisabled is "false" the output is as follows. (note the 'in thread' messages):

msg:'main thread (pre)' grant_type:password uri:http://localhost:8081
msg:'in thread' grant_type:password uri:http://localhost:8081
msg:'in thread' grant_type:password uri:password
msg:'in thread' grant_type:http://localhost:8081 uri:http://localhost:8081
msg:'main thread (post)' grant_type:password uri:http://localhost:8081
end

When propertyEditorDisabled is "true" the output is as follows. (note the 'in thread' messages):

msg:'main thread (pre)' grant_type:password uri:http://localhost:8081
msg:'in thread' grant_type:password uri:http://localhost:8081
msg:'main thread (post)' grant_type:password uri:http://localhost:8081
end
@lviggiano
Copy link
Collaborator

lviggiano commented Apr 23, 2020

👍
thanks for your investigation.

@lviggiano
Copy link
Collaborator

lviggiano commented Jun 6, 2020

You were right, the problem was exactly there. Issue introduced by commit 1acb314, in reference to #255 #254.

Duplicate of #268; fixed by 1f89055

It should be fixed now.

@lviggiano
Copy link
Collaborator

lviggiano commented Jun 7, 2020

I released 1.0.12, this should fix the issue. Let me know.

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

2 participants