[crac] RFR: Move more FD tracking to java layer [v2]
Anton Kozlov
akozlov at openjdk.org
Mon Jun 12 12:19:16 UTC 2023
On Mon, 12 Jun 2023 07:04:30 GMT, Radim Vansa <duke at openjdk.org> wrote:
>> Anton Kozlov has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 95 commits:
>>
>> - Merge remote-tracking branch 'jdk/crac/crac' into newfd
>> - Fix a bug in classPathMatching
>> - Remove misleading UnreachableSocket
>> - Merge remote-tracking branch 'jdk/crac/crac' into newfd
>> - Cleanup
>> - Remove AcceptingSocketResource
>>
>> Does not need to exist
>> - Cleanup and extend FileDescriptor nativeDesc
>> - Update
>> - Workaround for the blocking context
>> - Claim closing socket
>> - ... and 85 more: https://git.openjdk.org/crac/compare/a282698d...bcfd07a4
>
> src/java.base/share/classes/jdk/crac/impl/ExceptionHolder.java line 30:
>
>> 28: }
>> 29:
>> 30: public void reSuppress(Exception e) {
>
> Was thinking about a better name... `adoptSuppressed`?
I don't like the suggestion. What is the problem with the original name?
> src/java.base/share/classes/jdk/internal/crac/ClaimedFDs.java line 62:
>
>> 60: }
>> 61:
>> 62: public void claimFd(FileDescriptor fd, Object claimer, Supplier<Exception> supplier, Object... suppressedClaimers) {
>
> Knowing all potential claimers seems limiting, this would require breaking the encapsulation in a more complex chain.
> I suggest to change this to `claim(Object claimed, Object claimer, Suppler<Exception> supplier)` and then assume that `claimed == suppressedClaimer`. The implementation would handle the `claimed == claimer` case for FD a bit differently, but e.g. `ZipFile` would not claim FD itself, but only the `RandomAccessFile zfile`.
> All claims would be recorded (not overwriting anything) and the final claimant would be resolved only after all notifications complete.
In general, yes, that may work. But I want to postpone this until we find a case that will require the fix and do it with example in hands.
> src/java.base/unix/native/libjava/FileDescriptor_md.c line 117:
>
>> 115: }
>> 116:
>> 117: static const char* family2str(int family) {
>
> I don't think this belongs to FileDescriptor. You have tried to let Java part of FileDescriptor not know about its type, but only by moving socket-specific handling to native code which contradicts the title of this PR.
I can just delete this. I preserved this only because #43 introduced family reporting. Do you think this is useful?
-------------
PR Review Comment: https://git.openjdk.org/crac/pull/79#discussion_r1226575666
PR Review Comment: https://git.openjdk.org/crac/pull/79#discussion_r1226571724
PR Review Comment: https://git.openjdk.org/crac/pull/79#discussion_r1226574946
More information about the crac-dev
mailing list