Re: RFR: 8303485: Replacing os.name for operating system customization [v2]
> Improvements to support OS specific customization for JDK internal use: > - To select values and code; allowing elimination of unused code and values > - Optionally evaluated by build processes, compilation, or archiving (i.e. > CDS) > - Simple API to replace adhoc comparisons with `os.name` > - Clear and consistent use across build, runtime, and JDK modules > > The PR includes updates within java.base to use the new API. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Move OperatingSystem from jdk.internal.misc to jdk.internal.util Rename Mac -> MacOS; isMac() -> isMacOS() - Changes: - all: https://git.openjdk.org/jdk/pull/12931/files - new: https://git.openjdk.org/jdk/pull/12931/files/736ab3cc..342f30f2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12931=01 - incr: https://webrevs.openjdk.org/?repo=jdk=12931=00-01 Stats: 467 lines in 15 files changed: 210 ins; 235 del; 22 mod Patch: https://git.openjdk.org/jdk/pull/12931.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12931/head:pull/12931 PR: https://git.openjdk.org/jdk/pull/12931
Re: RFR: 8303485: Replacing os.name for operating system customization
On Fri, 10 Mar 2023 14:01:04 GMT, Alan Bateman wrote: >> Improvements to support OS specific customization for JDK internal use: >> - To select values and code; allowing elimination of unused code and values >> - Optionally evaluated by build processes, compilation, or archiving (i.e. >> CDS) >> - Simple API to replace adhoc comparisons with `os.name` >> - Clear and consistent use across build, runtime, and JDK modules >> >> The PR includes updates within java.base to use the new API. > > src/java.base/share/classes/jdk/internal/misc/OperatingSystem.java line 23: > >> 21: * questions. >> 22: */ >> 23: package jdk.internal.misc; > > I see there is a follow on PR to see this in several modules. We can't have > jdk.internal.misc exported widely as it contains Unsafe and several other > important classes. jdk.internal.util may be a better place, at the same of > turning that package into a place for random stuff. Agree. `jdk.internal.platform` can be one option. It can be extended to include platform-related utilities beyond containers. - PR: https://git.openjdk.org/jdk/pull/12931
Re: RFR: 8303809: Dispose context in SPNEGO NegotiatorImpl [v2]
> This patch fixes a possible native memory leak in case of a custom native GSS > provider. > The actual leak was reported in production. > > sun/security/jgss, sun/security/krb5, sun/net/www/protocol/http jtreg tests > are passed Alexey Bakhtin has updated the pull request incrementally with one additional commit since the last revision: Fix formatting - Changes: - all: https://git.openjdk.org/jdk/pull/12920/files - new: https://git.openjdk.org/jdk/pull/12920/files/8efa0c76..1b34252b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12920=01 - incr: https://webrevs.openjdk.org/?repo=jdk=12920=00-01 Stats: 7 lines in 3 files changed: 5 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/12920.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12920/head:pull/12920 PR: https://git.openjdk.org/jdk/pull/12920
Re: RFR: 8303809: Dispose context in SPNEGO NegotiatorImpl
On Fri, 10 Mar 2023 15:05:16 GMT, Weijun Wang wrote:
>> This patch fixes a possible native memory leak in case of a custom native
>> GSS provider.
>> The actual leak was reported in production.
>>
>> sun/security/jgss, sun/security/krb5, sun/net/www/protocol/http jtreg tests
>> are passed
>
> src/java.security.jgss/share/classes/sun/net/www/protocol/http/spnego/NegotiatorImpl.java
> line 134:
>
>> 132: } catch(Exception ex) {
>> 133: //dispose context silently
>> 134: }
>
> Why is this cleanup necessary here but not in `nextToken()`? If we don't do
> any cleanup here, will `disposeContext()` be called inside
> `HttpURLConnection`?
GSSContext could be allocated in init() line 97 but fails with Exception in
context.initSecContext(). In this case null Negotiator is returned
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/net/www/protocol/http/Negotiator.java#L71
to NegotiatorAuthenticator:
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/net/www/protocol/http/NegotiateAuthentication.java#L224.
So nobody can clean context from HttpURLConnection
-
PR: https://git.openjdk.org/jdk/pull/12920
Re: RFR: 8303965: java.net.http.HttpClient should reset the stream if response headers contain malformed header fields [v2]
On Fri, 10 Mar 2023 14:27:21 GMT, Daniel Fuchs wrote: >> According to RFC 7540: >> >> Endpoints MUST treat a request or response that contains >> undefined or invalid pseudo-header fields as malformed (Section 8.1.2.6). >> >> Section-8.1.2.6 Malformed requests or responses that are detected MUST be >> treated as a stream error (Section 5.4.2) of type PROTOCOL_ERROR. >> >> The current behavior is to close the connection with protocol error. This >> change makes it reset the stream instead. > > Daniel Fuchs has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - add bug id to test > - Merge branch 'master' into MalformedResponse-8303965 > - 8303965 HttpClient tests are stable and tier2 passed - PR: https://git.openjdk.org/jdk/pull/12976
Re: RFR: 8303809: Dispose context in SPNEGO NegotiatorImpl
On Wed, 8 Mar 2023 09:05:19 GMT, Alexey Bakhtin wrote:
> This patch fixes a possible native memory leak in case of a custom native GSS
> provider.
> The actual leak was reported in production.
>
> sun/security/jgss, sun/security/krb5, sun/net/www/protocol/http jtreg tests
> are passed
src/java.base/share/classes/sun/net/www/protocol/http/NegotiateAuthentication.java
line 254:
> 252: try {
> 253: negotiator.disposeContext();
> 254: }catch(IOException ioEx) {
Please add a space before `catch`.
src/java.security.jgss/share/classes/sun/net/www/protocol/http/spnego/NegotiatorImpl.java
line 134:
> 132: } catch(Exception ex) {
> 133: //dispose context silently
> 134: }
Why is this cleanup necessary here but not in `nextToken()`? If we don't do any
cleanup here, will `disposeContext()` be called inside `HttpURLConnection`?
-
PR: https://git.openjdk.org/jdk/pull/12920
Re: RFR: 8303809: Dispose context in SPNEGO NegotiatorImpl
On Wed, 8 Mar 2023 09:05:19 GMT, Alexey Bakhtin wrote: > This patch fixes a possible native memory leak in case of a custom native GSS > provider. > The actual leak was reported in production. > > sun/security/jgss, sun/security/krb5, sun/net/www/protocol/http jtreg tests > are passed Yes, you are right. I believe the reason is that once an 200 OK is received, no one cares to look at the token anymore. - PR: https://git.openjdk.org/jdk/pull/12920
Re: RFR: 8303965: java.net.http.HttpClient should reset the stream if response headers contain malformed header fields [v2]
> According to RFC 7540: > > Endpoints MUST treat a request or response that contains > undefined or invalid pseudo-header fields as malformed (Section 8.1.2.6). > > Section-8.1.2.6 Malformed requests or responses that are detected MUST be > treated as a stream error (Section 5.4.2) of type PROTOCOL_ERROR. > > The current behavior is to close the connection with protocol error. This > change makes it reset the stream instead. Daniel Fuchs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - add bug id to test - Merge branch 'master' into MalformedResponse-8303965 - 8303965 - Changes: - all: https://git.openjdk.org/jdk/pull/12976/files - new: https://git.openjdk.org/jdk/pull/12976/files/4e76e29d..101dbc64 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12976=01 - incr: https://webrevs.openjdk.org/?repo=jdk=12976=00-01 Stats: 66305 lines in 743 files changed: 57087 ins; 2356 del; 6862 mod Patch: https://git.openjdk.org/jdk/pull/12976.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12976/head:pull/12976 PR: https://git.openjdk.org/jdk/pull/12976
Re: RFR: 8303485: Replacing os.name for operating system customization
On Wed, 8 Mar 2023 19:15:16 GMT, Roger Riggs wrote: > Improvements to support OS specific customization for JDK internal use: > - To select values and code; allowing elimination of unused code and values > - Optionally evaluated by build processes, compilation, or archiving (i.e. > CDS) > - Simple API to replace adhoc comparisons with `os.name` > - Clear and consistent use across build, runtime, and JDK modules > > The PR includes updates within java.base to use the new API. > > Has this totally killed of BSD support on the JDK side? I thought building > > non-macOS BSD was still viable, but perhaps not - certainly not after this > > change. > > The macOS port in 7u4 was based on a BSD port and there was a time when it > was possible to build. Right now, we have src/hotspot/os/bsd and native code > in the libs that is compiled in with _ALLBSD_SOURCE. It's sad to see the BSD > port bit rot but if it's not maintained or tested in the OpenJDK project then > the changes proposed here may be okay. For porters then know which code needs > to be ported is important, ideally there would be a compile or build time if > someone significant is missing. Yes it is unfortunate. It would be nice if NetBSD, FreeBSD, OpenBSD, and friends would collaborate. I think the majority of their changes are fixing build issues and making AWT/Swing use X11 instead of macOS-specific stuff (Quartz or Cocoa or whatever). Maybe I'll go poke them eventually. - PR: https://git.openjdk.org/jdk/pull/12931
RFR: 8303965: java.net.http.HttpClient should reset the stream if response headers contain malformed header fields
According to RFC 7540: Endpoints MUST treat a request or response that contains undefined or invalid pseudo-header fields as malformed (Section 8.1.2.6). Section-8.1.2.6 Malformed requests or responses that are detected MUST be treated as a stream error (Section 5.4.2) of type PROTOCOL_ERROR. The current behavior is to close the connection with protocol error. This change makes it reset the stream instead. - Commit messages: - 8303965 Changes: https://git.openjdk.org/jdk/pull/12976/files Webrev: https://webrevs.openjdk.org/?repo=jdk=12976=00 Issue: https://bugs.openjdk.org/browse/JDK-8303965 Stats: 296 lines in 7 files changed: 191 ins; 70 del; 35 mod Patch: https://git.openjdk.org/jdk/pull/12976.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12976/head:pull/12976 PR: https://git.openjdk.org/jdk/pull/12976
Re: RFR: 8303485: Replacing os.name for operating system customization
On Fri, 10 Mar 2023 02:43:15 GMT, David Holmes wrote: >> Names and branding have changed over the years. >> It may be prudent to remove the name entirely and stick to a 'generic' >> identification of the OS as the Enum name. > > The current branding is "macOS" and we made a lot of changes in various > places (but not all of course) to ensure that we use "macOS". Even if this > can change in the future I think starting with the current terminology rather > than defining something new, would be much better. I think "macOS" would be better here. Yes, Apple have changed it several times but it has been stable since about 2016. - PR: https://git.openjdk.org/jdk/pull/12931
Re: RFR: 8303485: Replacing os.name for operating system customization
On Wed, 8 Mar 2023 19:15:16 GMT, Roger Riggs wrote: > Improvements to support OS specific customization for JDK internal use: > - To select values and code; allowing elimination of unused code and values > - Optionally evaluated by build processes, compilation, or archiving (i.e. > CDS) > - Simple API to replace adhoc comparisons with `os.name` > - Clear and consistent use across build, runtime, and JDK modules > > The PR includes updates within java.base to use the new API. src/java.base/share/classes/java/util/zip/ZipFile.java line 28: > 26: package java.util.zip; > 27: > 28: import jdk.internal.misc.OperatingSystem; You might want to move this to be with the other imports of jdk.internal. src/java.base/share/classes/jdk/internal/misc/OperatingSystem.java line 23: > 21: * questions. > 22: */ > 23: package jdk.internal.misc; I see there is a follow on PR to see this in several modules. We can't have jdk.internal.misc exported widely as it contains Unsafe and several other important classes. jdk.internal.util may be a better place, at the same of turning that package into a place for random stuff. - PR: https://git.openjdk.org/jdk/pull/12931
Re: RFR: 8303485: Replacing os.name for operating system customization
On Fri, 10 Mar 2023 00:08:57 GMT, Justin King wrote: > Has this totally killed of BSD support on the JDK side? I thought building > non-macOS BSD was still viable, but perhaps not - certainly not after this > change. The macOS port in 7u4 was based on a BSD port and there was a time when it was possible to build. Right now, we have src/hotspot/os/bsd and native code in the libs that is compiled in with _ALLBSD_SOURCE. It's sad to see the BSD port bit rot but if it's not maintained or tested in the OpenJDK project then the changes proposed here may be okay. For porters then know which code needs to be ported is important, ideally there would be a compile or build time if someone significant is missing. - PR: https://git.openjdk.org/jdk/pull/12931
Re: Proposal to add another option to configure the DNS cache
Hi Sergey, Could you tell us a bit about the problem you're trying to solve? In most deployments I would expect either a resolver cache, or a local caching DNS server. Either of these makes our InetAddress caching redundant. The InetAddress cache has a few problems: - it ignores the time-to-live (TTL) information provided by the DNS server; other caches usually cache the entries until the TTL expires, we have a fixed caching period. - it caches all negative lookup results for a fixed period of time; other caches cache NXDOMAIN replies which contain TTL information, and do not cache lookups that failed because of server and network errors - when the cache policy is set to NEVER, all requests for the same server are serialized - if 100 threads try to resolve the same server, 100 requests will be made one after another. However, with the default cache timeouts of 30/10 seconds, these problems are usually unnoticeable. I'd rather not extend the default caching periods; I've seen DNS responses with 30 second TTL, and if we cache the results longer than that, we might break someone's design. Regards, Daniel pt., 10 mar 2023 o 11:12 Sergey Bylokhov napisał(a): > > Hello > > I would like to discuss the possible improvement of the DNS cache which is > implemented in the shared > code in "InetAddress.java": > https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/net/InetAddress.java#L923 > and controlled by the "InetAddressCachePolicy.java" > https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/net/InetAddressCachePolicy.java#L32 > > At the moment this cache can be configured by the application using the > following two properties: >(1) "networkaddress.cache.ttl" - cache policy for successful lookups >(2) "networkaddress.cache.negative.ttl" - cache policy for negative lookups > > These properties are useful but have one limitation as the TTL value is > directly related to storing > the record in the cache. > > - If an application wants to always have up-to-date data, it can always > make requests to the > server. But in this case, "negative responses"/errors can break the > application. > - If an application wants to make fewer requests to the server, and the DNS > value changes rarely > it can set a long timeout. But obviously, this will lead to the record will > be rarely updated. And > if at the time of the update the failure happens - our cache will become > useless as we will cache a > negative response. > > It would be good to have benefits from both cases above without the negative > part. I propose to > implement the possibility to update/refresh entries in the cache periodically > without deleting them. > > This can be implemented by an additional property like > "networkaddress.extended.cache.ttl". > > Consider an example: the application may set the next values > "networkaddress.cache.ttl" = 60 and > "networkaddress.extended.cache.ttl" = 600. This will cache the record for 1 > minute and then, if the > next request is successful, re-cache the value for another 1 minute, and so > on. If the next request > is unsuccessful, it will fallback to the cached record for 9 minutes(10 > minutes from the last > successful lookup), and finally after a few attempts to update/re-cache the > value will be deleted > from the cache. > > Default behavior when this new property is not set will not be changed. > > Thoughts? > > -- > Best regards, Sergey.
Proposal to add another option to configure the DNS cache
Hello I would like to discuss the possible improvement of the DNS cache which is implemented in the shared code in "InetAddress.java": https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/net/InetAddress.java#L923 and controlled by the "InetAddressCachePolicy.java" https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/net/InetAddressCachePolicy.java#L32 At the moment this cache can be configured by the application using the following two properties: (1) "networkaddress.cache.ttl" - cache policy for successful lookups (2) "networkaddress.cache.negative.ttl" - cache policy for negative lookups These properties are useful but have one limitation as the TTL value is directly related to storing the record in the cache. - If an application wants to always have up-to-date data, it can always make requests to the server. But in this case, "negative responses"/errors can break the application. - If an application wants to make fewer requests to the server, and the DNS value changes rarely it can set a long timeout. But obviously, this will lead to the record will be rarely updated. And if at the time of the update the failure happens - our cache will become useless as we will cache a negative response. It would be good to have benefits from both cases above without the negative part. I propose to implement the possibility to update/refresh entries in the cache periodically without deleting them. This can be implemented by an additional property like "networkaddress.extended.cache.ttl". Consider an example: the application may set the next values "networkaddress.cache.ttl" = 60 and "networkaddress.extended.cache.ttl" = 600. This will cache the record for 1 minute and then, if the next request is successful, re-cache the value for another 1 minute, and so on. If the next request is unsuccessful, it will fallback to the cached record for 9 minutes(10 minutes from the last successful lookup), and finally after a few attempts to update/re-cache the value will be deleted from the cache. Default behavior when this new property is not set will not be changed. Thoughts? -- Best regards, Sergey.
status / plans for JDK-8275838 : java.net.http client does not work over unix domain sockets ?
Hello, Can you help me / guide me find out any information regarding JDK-8275838 : java.net.http client does not work over unix domain sockets ? Issue is: http client that comes with JDK does not work with UnixDomain sockets . This should make it much easier to implement CLI tooling on linux - that makes extensive use of unix sockets. Most prominent example is of course Docker Daemon. Right now you can't write a docker daemon client with JDK only - even though most/all things are implemented. I submitted it to OpenJDK bug tracker but I have no visibility there. Or I don't know where to look. Any help is appreciated :). I saw the video about network enhancements https://www.youtube.com/watch?v=GPmeFv8t66E_channel=Java . https://twitter.com/ieugen222/status/1603062524437757952 https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8275838 Regards, -- Eugen Stan +40770 941 271 / https://www.netdava.combegin:vcard fn:Eugen Stan n:Stan;Eugen email;internet:eugen.s...@netdava.com tel;cell:+40720898747 x-mozilla-html:FALSE url:https://www.netdava.com version:2.1 end:vcard
Re: RFR: 8303485: Replacing os.name for operating system customization
Let's please not kill generic BSD support if at all possible. There is
NetBSD, OpenBSD, FreeBSD, and DragonflyBSD. I know FreeBSD and NetBSD have
OpenJDK 19 and 17 respectively.
On Wed, Mar 8, 2023, 6:54 PM David Holmes wrote:
> On Wed, 8 Mar 2023 19:15:16 GMT, Roger Riggs wrote:
>
> > Improvements to support OS specific customization for JDK internal use:
> > - To select values and code; allowing elimination of unused code and
> values
> > - Optionally evaluated by build processes, compilation, or archiving
> (i.e. CDS)
> > - Simple API to replace adhoc comparisons with `os.name`
> > - Clear and consistent use across build, runtime, and JDK modules
> >
> > The PR includes updates within java.base to use the new API.
>
> I guess I'm surprised this hasn't been done long before now. :)
>
> Just a couple of drive by comments (I agree with comments made by others).
>
> Has this totally killed of BSD support on the JDK side? I thought building
> non-macOS BSD was still viable, but perhaps not - certainly not after this
> change.
>
> Thanks
>
> src/java.base/share/classes/jdk/internal/misc/OperatingSystem.java line 48:
>
> > 46: * For example,
> > 47: * {@snippet lang = "java":
> > 48: * if (OperatingSystem.current() == Windows) {
>
> Doesn't `Windows` need to be prefixed with `OperatingSystem` here? Ditto
> for dispatch example following.
>
> src/java.base/share/classes/jdk/internal/misc/OperatingSystem.java line
> 105:
>
> > 103: */
> > 104: @ForceInline
> > 105: public static boolean isMac() {
>
> suggestion: isMacOS
>
> -
>
> PR: https://git.openjdk.org/jdk/pull/12931
>
smime.p7s
Description: S/MIME Cryptographic Signature
