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

Serializing org.slf4j.Logger results in StackOverflowError #305

Closed
nimo23 opened this issue Aug 11, 2019 · 8 comments
Closed

Serializing org.slf4j.Logger results in StackOverflowError #305

nimo23 opened this issue Aug 11, 2019 · 8 comments
Labels
bug Something isn't working right
Milestone

Comments

@nimo23
Copy link

nimo23 commented Aug 11, 2019

With jackson I get the following json-string as a result of an empty bean with the setting mapper.disable(SerializationFeature.FAIL_ON_EMPTY_BEANS)

{ }

With yasson I get this stacktrace:

Caused by: java.lang.StackOverflowError
	at org.glassfish.javax.json@1.1.2//org.glassfish.json.JsonGeneratorImpl.writeString(JsonGeneratorImpl.java:608)
	at org.glassfish.javax.json@1.1.2//org.glassfish.json.JsonGeneratorImpl.writeString(JsonGeneratorImpl.java:614)
	at org.glassfish.javax.json@1.1.2//org.glassfish.json.JsonPrettyGeneratorImpl.writeIndent(JsonPrettyGeneratorImpl.java:108)
	at org.glassfish.javax.json@1.1.2//org.glassfish.json.JsonPrettyGeneratorImpl.writeComma(JsonPrettyGeneratorImpl.java:116)
	at org.glassfish.javax.json@1.1.2//org.glassfish.json.JsonGeneratorImpl.writeName(JsonGeneratorImpl.java:171)
	at org.glassfish.javax.json@1.1.2//org.glassfish.json.JsonGeneratorImpl.writeKey(JsonGeneratorImpl.java:489)
	at org.glassfish.javax.json@1.1.2//org.glassfish.json.JsonPrettyGeneratorImpl.writeKey(JsonPrettyGeneratorImpl.java:53)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.ObjectSerializer.marshallProperty(ObjectSerializer.java:88)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.ObjectSerializer.serializeInternal(ObjectSerializer.java:61)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.AbstractContainerSerializer.serialize(AbstractContainerSerializer.java:64)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.AbstractContainerSerializer.serializerCaptor(AbstractContainerSerializer.java:96)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.ObjectSerializer.marshallProperty(ObjectSerializer.java:103)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.ObjectSerializer.serializeInternal(ObjectSerializer.java:61)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.AbstractContainerSerializer.serialize(AbstractContainerSerializer.java:64)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.AbstractContainerSerializer.serializerCaptor(AbstractContainerSerializer.java:96)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.ObjectSerializer.marshallProperty(ObjectSerializer.java:103)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.ObjectSerializer.serializeInternal(ObjectSerializer.java:61)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.AbstractContainerSerializer.serialize(AbstractContainerSerializer.java:64)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.AbstractContainerSerializer.serializerCaptor(AbstractContainerSerializer.java:96)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.ObjectSerializer.marshallProperty(ObjectSerializer.java:103)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.ObjectSerializer.serializeInternal(ObjectSerializer.java:61)

I guess, the stacktrace is thrown because there are no accessors for that bean. In my opinion, if no accessors are found { } should be returned for that bean instead of StackOverflowError

@aguibert
Copy link
Member

hi @nimo23, are you sure there is not a circular dependency reference somewhere in your bean object? Can you share the contents of your java class?

If I try the following code, it works as expected:

public class AGGMain {

    public static class Bar {
        // no properties here
    }

    private static final Jsonb jsonb = JsonbBuilder.create();

    public static void main(String args[]) throws Exception {
        Bar empty = new Bar();
        System.out.println(jsonb.toJson(empty)); // prints: {}
    }
}

@nimo23
Copy link
Author

nimo23 commented Aug 12, 2019

I found the error, it's the log property. Use this class to reproduce:

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class TestBean {

	// The logger throws java.lang.StackOverflowError
	protected final Logger log = LoggerFactory.getLogger(getClass());

	private final String name;

	public TestBean(String name) {
		this.name = name;
	}

	public boolean nameIsSet() {
		return name != null;
	}
}

I get this stacktrace which differs a little from the above:

Caused by: java.lang.StackOverflowError
	at java.base/java.util.concurrent.ConcurrentHashMap$BaseIterator.<init>(ConcurrentHashMap.java:3426)
	at java.base/java.util.concurrent.ConcurrentHashMap$ValueIterator.<init>(ConcurrentHashMap.java:3467)
	at java.base/java.util.concurrent.ConcurrentHashMap$ValuesView.iterator(ConcurrentHashMap.java:4732)
	at org.eclipse.yasson//org.eclipse.yasson.internal.ComponentMatcher.searchComponentBinding(ComponentMatcher.java:192)
	at org.eclipse.yasson//org.eclipse.yasson.internal.ComponentMatcher.getSerializerBinding(ComponentMatcher.java:148)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.SerializerBuilder.build(SerializerBuilder.java:73)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.ObjectSerializer.marshallProperty(ObjectSerializer.java:102)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.ObjectSerializer.serializeInternal(ObjectSerializer.java:61)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.AbstractContainerSerializer.serialize(AbstractContainerSerializer.java:64)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.AbstractContainerSerializer.serializerCaptor(AbstractContainerSerializer.java:96)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.ObjectSerializer.marshallProperty(ObjectSerializer.java:103)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.ObjectSerializer.serializeInternal(ObjectSerializer.java:61)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.AbstractContainerSerializer.serialize(AbstractContainerSerializer.java:64)
	at org.eclipse.yasson//org.eclipse.yasson.internal.serializer.AbstractContainerSerializer.serializerCaptor(AbstractContainerSerializer.java:96)

If I delete the log property all works.

If I use mapper.disable(SerializationFeature.FAIL_ON_EMPTY_BEANS), jackson will see the log-property as an empty bean and will not try to (de)serialize it, so I guess, jacksons definition of what is an emtpy bean is a little larger than yassons definition of an empty bean.

@aguibert
Copy link
Member

normally Loggers are defined as static, and static fields will not be included as a bean property

@nimo23
Copy link
Author

nimo23 commented Aug 12, 2019

Imagine defining a logger within an abstract class and each extended class has a logger instance with its class name (getClass()), this is common. Also, in 3rd party libs, a Logger can be defined without using static (for example, within an abstract class) and we cannot use @JsonBTransient on 3rd party libs. Jackson (with disabled FAIL_ON_EMPTY_BEANS) does generally not include the logger instance variable within json (de)serializing which makes sense.

Should not be matter , if static logger or not..yasson should not try to (de)serialize a logger instance.

@aguibert
Copy link
Member

I tried this out locally and got the same StackOverflowError. While I still think non-static Loggers is bad practice, Yasson should tolerate this scenario better than it currently does.

@aguibert aguibert added the bug Something isn't working right label Aug 13, 2019
@nimo23
Copy link
Author

nimo23 commented Aug 13, 2019

Yes, that would be good.

Btw, could you please look at #306 again and think about to re-open. Would be great.

@jbescos
Copy link
Member

jbescos commented Nov 5, 2019

Shouldn't be transient the logger instance?:
protected final transient Logger log = LoggerFactory.getLogger(getClass());

Then you can reuse the logger in subclasses, and the json parser should skip the log from serialization.

@aguibert aguibert added this to the 1.0.6 milestone Nov 5, 2019
@aguibert aguibert changed the title StackOverflowError error on empty beans? Serializing org.slf4j.Logger results in StackOverflowError Nov 5, 2019
@aguibert
Copy link
Member

this issue should now be resolved by PR #359

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working right
Projects
None yet
Development

No branches or pull requests

3 participants