[QUESTION] Suppressed IOException/SQLServerException when using ResultSet.getClob(...)

[dru1]

Question

After updating the mssql-jdbc driver from version 6.2.2.jre8 to 7.4.1jre8 for our company application, we have noticed an increased number of suppressed IOExceptions and SQLServerExceptions. This behavior appears to be caused within the driver when java.sql.ResultSet.getClob() is called.

Our company application uses Hibernate 5.4.10.Final for data access and some of our entities use an @Lob mapping pointing to NVARCHAR(max) columns in the database. Whenever such entities are loaded from the database, Hibernate calls java.sql.ResultSet.getClob() at some point, resulting in these suppressed exceptions.

From the application's point of view, there are no visible errors because these exceptions are suppressed, but there is a performance loss for data access due to throwing and suppressing exceptions. This performance loss has been confirmed with profiling software. Is there a specific reason why this is happening? Did we miss something in the driver configuration?

We tested this behavior with Java 8 on Windows 10 with SQL Server 2014 and 2019 using driver versions 9.2.1jre8 and 7.4.1jre8. To confirm this behavior, we also created a test project to try different driver versions.

The test project can be found here:
https://github.com/dru1/mssql-jdbc-clob-exception

The test project does not use Hibernate, just the driver itself and some JDBC code derived from this guide:
https://docs.microsoft.com/en-us/sql/connect/jdbc/step-3-proof-of-concept-connecting-to-sql-using-java?view=sql-server-ver15

[karinazhou]

Hi @dru1 ,

Thank you for bringing this up. If I understand it correctly, the suppressed exceptions start showing up since 7.4.1jre8 and existing in all the following versions, right?

We will test it locally with your test project and see why this change of behavior happens.

Thanks,

[dru1]

Hi @karinazhou,

maybe the suppressed exceptions show up in an earlier version too. I've just tried the version 7.2.2.jre8 and 7.0.0.jre8 which seem to have this behavior too. The version 6.4.0.jre8 does not throw the suppressed exceptions according to the test project. I hope this helps to narrow it down.

Thanks

[karinazhou]

Hi @dru1 ,

Some updates on this issue. After testing your repo application, the exception starts shown up in version 6.5.2-preview. I went through the commits in that driver version and find this PR introduced the suppressed exception. I will dive deeper into this PR to see why this starts happening.

Thanks,

[karinazhou]

@dru1
After more investigation, the cause of this suppressed exception is from the unexpected closure of the active stream.

In your test application, when reading the stream from the resultSet object, the active stream from resultSet.getClob(column).getCharacterStream()) is passed as a reference input to readFully method.
https://github.com/dru1/mssql-jdbc-clob-exception/blob/master/src/main/java/org/example/ClobExceptionTester.java#L152

The try block with BufferedReader in the readFully method will close the currently active stream (set its tdsReader to null) when it is done.
https://github.com/dru1/mssql-jdbc-clob-exception/blob/master/src/main/java/org/example/ClobExceptionTester.java#L182

However, this is not supposed to happen while the SQLServerClob object is still active. After version 6.2.2, we change the logic in the driver how we handle LOB streaming. The driver keeps activeStreams throughout the lifecycle of SQLServerClob object. When the resultSet.next() is called again after the readFully method, it hits the following exception when trying to reset the stream:
https://github.com/microsoft/mssql-jdbc/blob/dev/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerClob.java#L376

    private void getStringFromStream() throws SQLServerException {
        if (null == value && !activeStreams.isEmpty()) {
            BaseInputStream stream = (BaseInputStream) activeStreams.get(0);
            try {
                stream.reset();
            } catch (IOException e) {
                SQLServerException.makeFromDriverError(con, null, e.getMessage(), null, false);
            }
            Charset cs = (defaultCharset == null) ? typeInfo.getCharset() : defaultCharset;
            value = new String(stream.getBytes(), cs);
        }
    }

mssql-jdbc/src/main/java/com/microsoft/sqlserver/jdbc/SimpleInputStream.java

Line 93 in 42a7adc

final void checkClosed() throws IOException {

    final void checkClosed() throws IOException {
        if (null == tdsReader)
            throw new IOException(SQLServerException.getErrString("R_streamIsClosed"));
    }

If you avoid using the BufferedReader in readFully method and read the content directly from the Reader, it won't give you the suppressed exception about Filling Lobs before closing: The stream is closed.

@Nonnull
    private String readFullyDirectly(@Nonnull Reader reader) throws IOException {
           StringBuilder content = new StringBuilder();
            int value;
            while ((value = reader.read()) != -1) {
                content.append((char) value);
            }
            return content.toString();
    }

Please feel free to give it a try and see how it works.

Thanks,

[dru1]

Hello @karinazhou,

thank you very much for your detailed analysis. I can confirm that if the Reader remains open after reading, no SQLServerException is thrown and suppressed. However, since it is an acquired instance of Reader, it is also a Closable that must be closed at some point by a close() call to avoid a resource leak, correct?

Unfortunately, the documentation of Clob#getCharacterStream does not contain any hint to do this or not, but as far as I know, it is a common practice to close acquired instances by a try-with-resources block. The static code analysis tool SonarSource has a rule for this: RSPEC-2095, although for some reason it does not mark the affected code as vulnerable. I'm not sure if it's safe to avoid the close() call, as other JDBC drivers may rely on it. Do you have more information on this topic?

And even if it is safe to avoid the close() call, it wouldn't help us much since the Reader instance is acquired and closed internally in the Hibernate ORM framework when reading the @Lob mapping on a Clob. The question remains if this is an invalid configuration on our end or if it turns out that closing the Reader is wrong, even a bug in Hibernate.

What do you think about this?

[karinazhou]

Hi @dru1 ,

Good opinions regarding this topic. I agree that our documentation doesn't illustrate the behavior when getCharacterStream method is called on Clob objects. Same behavior for Blob objects when getBinaryStream is called. Even for myself, I have to go through a bunch of old PRs and issues to see why this is implemented in this way.

There were some discussions back in 2017 about giving access to the InputStream even after the resultSet or the statement is closed. This design is to avoid loading entire data into memory for large content (#673 / #567 / #697 ). When LOB objects are closed, these InputStreams will be closed too. See the free methods below.
https://github.com/microsoft/mssql-jdbc/blob/dev/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java#L119
https://github.com/microsoft/mssql-jdbc/blob/dev/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerClob.java#L169

So the InputStreams will be closed properly. I am not quite familiar with how the Hibernate ORM framework deals with LOB objects but I think it needs to avoid closing the InputStreams before the LOB objects are closed.

[karinazhou]

Hi @dru1 ,

We are closing this issue due to no actions required. Please feel free to request us to reopen it if needed.

Thanks!