Input streams returned by DecompressingEntity.getContent() should support mark(int) and reset()

[garydgregory]

Input streams returned by DecompressingEntity.getContent() should support mark(int) and reset() if the underlying stream does

• Pass calls to the underlying InputStream for mark(int)
• Pass calls to the underlying InputStream for reset()
• LazyDecompressingInputStream extends FilterInputStream
• Reimplement internals fluently
• Reuse Closer.close(Closeable)
• Tested locally with git master 5.3-beta2-SNAPSHOT

[arturobernalg]

Shouldn't we use ReentrantLock instead?

[ok2c]

@garydgregory No more synchronized for us going forward. Thread synchronization must be implemented using locks in order to be compatible with Java 21 fibers

[garydgregory]

Hm, I guess the whole class isn't thread-safe to start with due to initWrapper(). I'll rework the PR.

These synchronized come from the Java 8, 11, and 17 contracts.

[garydgregory]

Thank you both for the reviews.

The simplest change and implementation that keeps in line with what to have today is to remove the synchronized keyword and let the delegate's implementation do what it will. If we wanted mark/reset to be thread-safe, we could make the whole class thread-safe like this:

   private final ReentrantLock lock = new ReentrantLock();
   ...
    /**
     * Thread-safe initialization of the wrapped stream.
     *
     * @return the wrapper stream.
     * @throws IOException if an IO error occurs.
     */
    private InputStream initWrapper() throws IOException {
        if (wrapperStream == null) {
            lock.lock();
            try {
                wrapperStream = wrapperStream != null ? wrapperStream : inputStreamFactory.create(in);
            } finally {
                lock.unlock();
            }
        }
        return wrapperStream;
    }

but then each call to the stream would lock-unlock.

So, either:

• Don't use synchronized and that's it.
• Only have mark/reset use lock-unlock, or
• Implement using the above thread-safe initWrapper()?

For now, the PR takes the simplest route.
WDYT?

[ok2c]

• Don't use synchronized and that's it.

I would go for this one.

[arturobernalg]

same here

[garydgregory]

OK, I merged with this option.

[ok2c]

@garydgregory I am not sure synchronization is even needed here. This class is not supposed to be used for concurrent access.