[CONJ-171] Removed Sometimes Unnecessary String Instantiation That Can Cause Thread Blocking Created: 2015-07-23  Updated: 2015-09-02  Resolved: 2015-07-23

Status: Closed
Project: MariaDB Connector/J
Component/s: Other
Affects Version/s: 1.1.8, 1.1.9, 1.2.0
Fix Version/s: 1.2.2

Type: Task Priority: Minor
Reporter: Diego Dupin Assignee: Diego Dupin
Resolution: Fixed Votes: 0
Labels: None


 Description   

report from https://github.com/MariaDB/mariadb-connector-j/pull/23 from andysenn

Resolved an issue where several concurrent reads of large byte arrays from a result set could cause threads to back up in a synchronization queue.

Calling #getBytes(...) on the result set makes a call to #getValueObject(...), which calls AbstractValueObject#isNull(). The existing workflow was as follows:

1) Declare 'rawValue' and set it to the return value of #getString() <- This can cause issues as described below
2) Declare 'zeroTimestamp' and set it
3) Declare 'zeroDate' and set it
4.1) If 'rawBytes' is null, return true
4.2) If the datatype is either TIMESTAMP or DATETIME, check for equality between 'rawValue' and 'zeroTimestamp'. If true, return true
4.3) If the datatype is DATE, check for equality between 'rawValue' and 'zeroDate'. If true, return true
4.4) Otherwise, return false

In this workflow, we make a call to #getString() whether we end up needing its return value or not.

Calling #getString() executes a chain of method calls that eventually end up at java.nio.charset.CoderResult$Cache.get(int). In the middle of this call stack is a call from java.lang.StringCoding to a statically referenced instance of java.lang.StringCoding$StringDecoder. Transitively, this means that it ends up using the same instance of java.nio.charset.CoderResult$Cache. Since $Cache#get(int) is synchronized, this can cause threads to back up in a high-traffic environment.

This issue is exacerbated by the fact that it is run within the context of an open database connection. If using database connection pooling, enough threads can pile up in the synchronization pool that the database connection pool is effectively exhausted. Requests for database connections can continue to enqueue until the JVM exhausts its heap and becomes unstable.

Since the return value of #getString() is only used when the data type is DATE, DATETIME, or TIMESTAMP, its invocation is unnecessary in other cases. Instead of calling it unconditionally, I replaced the occurrences of 'rawValue' in the conditions with calls to #getString(). Since the data type will either be DATE or (DATETIME or TIMESTAMP), it would not be possible for the invocation to occur twice as that condition would short-circuit the && condition of zeroXXX.equals(rawValue).



 Comments   
Comment by Diego Dupin [ 2015-07-23 ]

git pull request :
https://github.com/MariaDB/mariadb-connector-j/commit/59a38e3a2b1412c2c3a7d69eafcfb93239cc2707

Generated at Thu Feb 08 03:13:42 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.