JDK-8195803 : Eliminate use of sun.nio.ch.DirectBuffer in javafx.media
  • Type: Bug
  • Component: javafx
  • Sub-Component: media
  • Affected Version: 9,10,openjfx11
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2018-01-19
  • Updated: 2018-06-13
  • Resolved: 2018-02-23
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
openjfx11Fixed
Related Reports
Blocks :  
Relates :  
Description
The javafx.media module has a dependency on sun.nio.ch.DirectBuffer and jdk.internal.ref.Cleaner, both of which are internal classes. The following code is to blame:

        public void closeConnection() {
            ...
            if (buffer instanceof DirectBuffer) {
                ((DirectBuffer) buffer).cleaner().clean();
            }
        }

In an effort to eliminate the use of internal packages from core module by javafx.* modules we need an alternative solution.
Comments
http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/861a79b403da
23-02-2018

The .01 webrev looks good. +1
23-02-2018

http://cr.openjdk.java.net/~almatvee/8195803/webrev.01/ 1. Agree. Fixed. 2. Agree. Fixed. 3. This code is required in cases when we read less data then requested. gst_buffer_new_allocate() sets buffer size to length and if we read < length we need to update buffer size to indicate amount of valid data. This is needed mostly for MP3. Other formats actually will know how much data is left in file and thus should not request more then we have, but MP3 will do. Note: This code was never executed, due to bug for such case. With read < length we will re-enter while loop and since no more data is left we will return EOS, thus dropping last buffer. I fixed it. See line 831-835.
17-02-2018

1. The following block looks more complicated than it needs to be: 820 if (length <= MAX_READ_SIZE) 821 { 822 toRead = length; 823 } 824 else 825 { 826 if ((length - read) >= MAX_READ_SIZE) 827 toRead = MAX_READ_SIZE; 828 else 829 toRead = (length - read); 830 } I think the first "if" test is unnecessary. Since length isn't being decremented, it doesn't really need to be tested each time through the loop. The "if ((length - read) >= MAX_READ_SIZE)" will cover that case anyway, since read is initialized to 0. You might consider simplifying the above to: if ((length - read) >= MAX_READ_SIZE) toRead = MAX_READ_SIZE; else toRead = (length - read); 2. The following seems wrong (although I realize it matches the existing logic) if (size > 0 || size <= toRead) Shouldn't that be "&&" to be a proper range check? 3. Can you explain the following? 847 gst_buffer_set_size(buf, read); // Set real size
15-02-2018

Please review the following: http://cr.openjdk.java.net/~almatvee/8195803/webrev.00/ Reviewers: kcr This code was introduced by JDK-8093636. Fix is re-done to avoid large reads in Java.
10-02-2018

Once this dependency has been eliminated, then the qualified export can be removed from dependencies/java.desktop/module-info.java.extra and build.gradle. See the "REMOVING QUALIFIED EXPORTS" comment in JDK-8195798 for instructions.
23-01-2018