[Bug 64542] New: FileBackedDataSource closing external FileChannel

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug 64542] New: FileBackedDataSource closing external FileChannel

Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64542

            Bug ID: 64542
           Summary: FileBackedDataSource closing external FileChannel
           Product: POI
           Version: unspecified
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: POIFS
          Assignee: [hidden email]
          Reporter: [hidden email]
  Target Milestone: ---

I am creating POIFSFileSystem instances in various ways, depending on what I
have available: a File, FileChannel or InputStream. I just found out that
calling POIFSFileSystem.close() (via a try-with-resources) also closes the
FileChannel, even when that channel is passed to it from an external source.
Any subsequent operations on that channel (like calculating an MD5 hash) then
run into a ClosedChannelException. IMHO POIFSFileSystem/FileBackedDataSource
should only close the FileChannel if it has created it itself. It would be
great if you could fix this.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64542] FileBackedDataSource closing external FileChannel

Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64542

--- Comment #1 from PJ Fanning <[hidden email]> ---
Hi Arjohn,

I'm not sure this is an easy change and the code has been like this for a long
time - so there are risks that people rely on the behaviour as it is.

You might be able to wrap your FileChannel with a wrapper that delegates all
calls to the wrapped channel but the wrapper could have a no-op close().

If you feel strongly about this issue, maybe you could submit a patch. If so, I
would recommend adding a new constructor to FileBackedDataSource that takes a
flag that says not to close the FileChannel (or a set method).

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64542] FileBackedDataSource closing external FileChannel

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64542

--- Comment #2 from Arjohn Kampman <[hidden email]> ---
Hi PJ,

I have tried the wrapper approach, but unfortunately the close() method is
declared "final" in superclass AbstractInterruptibleChannel.

Maybe a third constructor variant for FileChannel would be an option? E.g.: new
POIFSFileSystem(FileChannel channel, boolean readOnly, boolean closeChannel)

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64542] FileBackedDataSource closing external FileChannel

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64542

--- Comment #3 from PJ Fanning <[hidden email]> ---
Yes - a third constructor - that allows the existing constructors to retain
their existing behaviours - makes sense.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64542] FileBackedDataSource closing external FileChannel

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64542

--- Comment #4 from Arjohn Kampman <[hidden email]> ---
Do you want me to supply a patch for this? Looks pretty simple to add.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64542] FileBackedDataSource closing external FileChannel

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64542

--- Comment #5 from PJ Fanning <[hidden email]> ---
Feel free to submit a patch.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64542] FileBackedDataSource closing external FileChannel

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64542

--- Comment #6 from Arjohn Kampman <[hidden email]> ---
Created attachment 37322
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37322&action=edit
Patch for POIFS ticket 64542

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64542] FileBackedDataSource closing external FileChannel

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64542

--- Comment #7 from PJ Fanning <[hidden email]> ---
Thanks for the patch but could you add a unit test?

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]