StackOverflowError from StringHelper

Description

Since upgrading from Hibernate 5.2.0 to 5.4.0.Final recently, we've been occasionally running into a StackOverflowError coming from org.hibernate.internal.util.StringHelper#replace. Unfortunately, this has been hard to reproduce at will to figure out the nature of the inputs triggering this condition.

Example:

Environment

Java 8, hibernate-core-5.4.0.Final, MySql 5.7,

Activity

Show:
Chris Cranford
February 8, 2019, 8:30 PM

I am afraid we will need more context than this to determine the root-cause. Are you able to provide us with the details of where in the Hibernate code is the call ingot StringHelper#replace at least happening?

While certain input values could make #replace problematic as a corner case bug, I am not seeing any recent changes to the helper class that stand out to me as a reason why you would observe differing behavior between 5.2 and 5.4; at least based on the helper class logic. Perhaps a call reference was changed/added. Knowing the initially call point where you observe this could at least give us some clue.

Guillaume Smet
February 8, 2019, 9:02 PM

I think your best bet is to modify Hibernate source code to catch the error, log the parameters passed to the method and throw the error.

That's probably the only way we will be able to understand what's going on (not sure we will have all the necessary information but that would be a good first step).

Joe Bumbaca
February 12, 2019, 8:03 PM
Edited

I was able to capture the arguments causing the StackOverflowError in the StringHelper#replace method. The call is originating from QueryParameterBindingsImpl#expandListValuedParameters (line 640).

It seems to be related to the "in" clause of the query containing many values (in the 1000s).

I've noticed that the issue does not occur all of the time. After some debugging, I think it may be related to the iteration order of the entry set of QueryParameterBindingsImpl#parameterListBindingMap in the QueryParameterBindingsImpl#expandListValuedParameters method.

For example, suppose there are two entries in the map, representing values of the two "in" clauses of the query. One of those "in" clauses has many values (1000s), while the other only has a few values. It seems that we only get the error when the entry with the many values is iterated first, which causes StringHelper#replace to be called with a very large expansion list. The placeholder in the "in" clause is replaced with these expanded values, which turn out to be more placeholders. Then on the second iteration of the loop, StringHelper#replace is called for the other "in" clause. But now, the afterPlaceholder argument is huge, which seems to cause the overflow.

When the iteration order is opposite, the StringHelper#replace method is called with the large expansion list on the last iteration of the loop so there is not a subsequent call to StringHelper#replace with the afterPlaceholder argument containing all of those placeholder values (i.e. $1).

I've decided to modify the underlying query so we don't create an "in" clause with a ton of values. We can get by with doing some filtering on the client side in order to avoid the potential overflow error. So I think we are good, and there's no need to address this just for us.

Just for completion, and in case you are curious, here are the values passed to StringHelper#replace, which is called in QueryParameterBindingsImpl#expandListValuedParameters. I'm attaching the afterPlaceholder value as a file since it is such a large string.

String beforePlaceholder = "select auditLogEntity\n" +
"from AuditLogEntity auditLogEntity\n" +
"where auditLogEntity.masterEntityClassName in ("
String placeholder = "?1"
String replacement = "?1, ?12202"
boolean wholeWords = true
boolean encloseInParensIfNecessary = true

Thank you.

Sanne Grinovero
August 14, 2019, 8:18 AM

The pulll request seems valid but it raises more questions: let’s discuss on the PR.

 

Sanne Grinovero
August 14, 2019, 8:29 AM

Ok my bad my IDE fooled me by letting me think this method was never used.

Fix looks good! It’s quite concerning that we use recursion at all for such a thing, but with ORM6 being on track to rewrite this all I’ll apply the patch and move.

Big thanks also for providing a good unit test!

 

Fixed

Assignee

Unassigned

Reporter

Jithu Menon

Fix versions

Labels

None

backPortable

Backport?

Suitable for new contributors

None

Requires Release Note

None

backportDecision

None

Worked in

5.2.0

Components

Affects versions

Priority

Major
Configure