[
https://issues.apache.org/jira/browse/HADOOP-20067?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18138464#comment-18138464
]
ASF GitHub Bot commented on HADOOP-20067:
-----------------------------------------
LuciferYang opened a new pull request, #8931:
URL: https://github.com/apache/hadoop/pull/8931
### Description of PR
JIRA: https://issues.apache.org/jira/browse/HADOOP-20067
`LocalFSFileInputStream#getAsyncChannel()` checks `asyncChannel == null`
outside the lock but not again inside it, and the field is not volatile. When
two threads make the first `readVectored()` call on the same stream, both can
open an `AsynchronousFileChannel`. `close()` only closes the one left in the
field, so the other keeps its file descriptor until the JVM exits.
`PositionedReadable` requires positioned reads, `readVectored` included, to be
thread-safe.
This patch makes the field volatile and checks it again inside the
`synchronized` block. It follows up on
https://github.com/apache/hadoop/pull/8511#discussion_r4452981403. A
`readVectored()` after `close()` still opens a new channel; that is unchanged
here.
### How was this patch tested?
Added
`TestRawLocalFileSystemAsyncChannel#testConcurrentGetAsyncChannelOpensSingleChannel`:
eight threads released from a barrier call `getAsyncChannel()` on a fresh
stream and must all get the same channel, which must be closed after `close()`.
With the fix this is deterministic. Without it the test fails when the threads
race, which depends on scheduling, so the race runs three times.
### For code changes:
- [x] Does the title of this PR start with the corresponding JIRA issue id
(e.g. 'HADOOP-17799. Your PR title ...')?
- [ ] Object storage: Have the integration tests been executed and the
endpoint declared according to the connector-specific documentation?
- [ ] If adding new dependencies to the code, are these dependencies
licensed in a way that is compatible for inclusion under [ASF
2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`,
`NOTICE-binary` files?
### AI Tooling
- [x] The PR includes the phrase "Contains content generated by Claude Opus".
- [x] My use of AI contributions follows the ASF legal policy
https://www.apache.org/legal/generative-tooling.html
Contains content generated by Claude Opus.
> RawLocalFileSystem: broken double-checked locking in
> LocalFSFileInputStream#getAsyncChannel() can leak AsynchronousFileChannels
> -------------------------------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-20067
> URL: https://issues.apache.org/jira/browse/HADOOP-20067
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Reporter: Yang Jie
> Priority: Minor
>
> LocalFSFileInputStream#getAsyncChannel() checks {{asyncChannel == null}}
> outside the lock but not again inside it, and the field is not volatile. When
> two threads make the first readVectored() call on the same stream, both can
> open an AsynchronousFileChannel. The last assignment wins and close() only
> closes that channel, so the other one keeps its file descriptor open until
> the JVM exits. PositionedReadable requires positioned reads, readVectored
> included, to be thread-safe.
> The fix makes the field volatile and checks it again inside the synchronized
> block. This was pointed out in the review of HADOOP-19901:
> https://github.com/apache/hadoop/pull/8511#discussion_r4452981403. A
> readVectored() call after close() still opens a new channel; this change does
> not touch that.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
