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.

Sign up for GitHub

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

Jump to bottom

[Java] FlightSQL JDBC DatabaseMetaData cache isn't thread-safe #38737

Closed
jduo opened this issue Nov 15, 2023 · 1 comment · Fixed by #38739
Closed

[Java] FlightSQL JDBC DatabaseMetaData cache isn't thread-safe #38737

jduo opened this issue Nov 15, 2023 · 1 comment · Fixed by #38739
Assignees
@jduo
Labels
Component: FlightRPC Component: Java Type: bug
Milestone
15.0.0

Comments

@jduo
Copy link
Member

jduo commented Nov 15, 2023

Describe the bug, including details regarding any error messages, version, and platform.

The cache of SqlInfo properties in the JDBC driver isn't thread-safe.

arrow/java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowDatabaseMetadata.java

Line 732 in 563078f

if (cachedSqlInfo.isEmpty()) {

If one thread starts populating the cache, and another thread requests a property while the cache is still populating, the second thread will see the cache is non-empty and think the cache has been fully populated when it might just have started populating. This will result in null properties values being retrieved and NPE errors.

Component(s)

FlightRPC, Java
@jduo jduo added the Type: bug label Nov 15, 2023
jduo added a commit to Bit-Quill/arrow that referenced this issue Nov 15, 2023
@jduo
apacheGH-38737: [Java] Fix JDBC caching of SqlInfo values
e86ace6 
Fix the thread-safety of DatabaseMetaData functions.
The cache of SqlInfo properties wasn't being populated in
a thread-safe manner.
@jduo
Copy link
Member Author

jduo commented Nov 15, 2023

This issue might the same as #38253

@github-actions github-actions bot mentioned this issue Nov 15, 2023
Merged
lidavidm pushed a commit that referenced this issue Nov 16, 2023
@jduo
GH-38737: [Java] Fix JDBC caching of SqlInfo values (#38739)
5583dbe 
### Rationale for this change
The cache of SqlInfo properties that ArrowDatabaseMetaData maintains isn't populated in a thread-safe way. This can cause JDBC applications trying to retrieve several properties from DatabaseMetaData to encounter missing properties when they shouldn't.

### What changes are included in this PR?
- Changed the checking for the cache being populated to be based on an AtomicBoolean marking that the cache is fully populated, rather than just checking if the cache is empty.
- Avoid having multiple threads call getSqlInfo() if they see that the cache is empty concurrently.

### Are these changes tested?
Verified existing unit tests.

### Are there any user-facing changes?
No.
* Closes: #38737

Authored-by: James Duong <james.duong@improving.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
@lidavidm lidavidm added this to the 15.0.0 milestone Nov 16, 2023
@YuriyGavrilov YuriyGavrilov mentioned this issue Dec 4, 2023
Closed
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
@jduo @dgreiss
apacheGH-38737: [Java] Fix JDBC caching of SqlInfo values (apache#38739)
9923758 
### Rationale for this change
The cache of SqlInfo properties that ArrowDatabaseMetaData maintains isn't populated in a thread-safe way. This can cause JDBC applications trying to retrieve several properties from DatabaseMetaData to encounter missing properties when they shouldn't.

### What changes are included in this PR?
- Changed the checking for the cache being populated to be based on an AtomicBoolean marking that the cache is fully populated, rather than just checking if the cache is empty.
- Avoid having multiple threads call getSqlInfo() if they see that the cache is empty concurrently.

### Are these changes tested?
Verified existing unit tests.

### Are there any user-facing changes?
No.
* Closes: apache#38737

Authored-by: James Duong <james.duong@improving.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jduo jduo

Labels
Component: FlightRPC Component: Java Type: bug
Projects
None yet
Milestone
15.0.0
Development

Successfully merging a pull request may close this issue.

GH-38737: [Java] Fix JDBC caching of SqlInfo values Bit-Quill/arrow
2 participants
@lidavidm @jduo