[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