Is SharedSecrets thread-safe?
Hello,
the class `jdk.internal.access.SharedSecrets` provides getter methods which all
look similar to this:
```
if (static_field == null) {
initialize();
}
return static_field;
```
However, neither the static fields are `volatile` nor are the getter methods
synchronized. So if my
understanding of the Java Memory Model is correct, the compiler is free to
reorder the two static
field reads. So it is in theory possible that the first read yields a
non-`null` value, but the
second read yields a `null` value which leads to incorrect behavior (for
further reading [1]).
It is probably rather unlikely that this actually happens because
`SharedSecrets` is in most cases
accessed from static initializers (which are only run once) and because not
many classes access the
same `SharedSecrets` fields.
Is this analysis correct or did I forget to consider parts of the Memory Model
logic, or is there
some JVM magic I am missing?
Kind regards
[1]
https://shipilev.net/blog/2016/close-encounters-of-jmm-kind/#wishful-benign-is-resilient
Re: Is SharedSecrets thread-safe?
That would also be my understanding of the current situation, though this
contradicts what
Claes wrote.
Maybe the JVM behaves in a way which does not allow reordering, but the JLS
definitely seems
to allow it. Section § 12.2.4 [0] only mentions that for the class to be
initialized there
has to exist a lock LC (or at least the happens-before relationship), but there
is no
"freeze the world" or anything similar which would force a happens-before
relationship
for the code in `SharedSecrets`.
Maybe most of the `SharedSecrets` methods are thread-safe (albeit extremely
brittle) because
the classes to which the accessor objects belong to have previously already
been loaded
before `SharedSecrets` is used, therefore having already established a
happens-before
relationship.
However, this is definitely not the case for all of the methods as shown by the
following
example:
```
CookieHandler.setDefault(new CookieHandler() {
@Override
public void put(URI uri, Map> responseHeaders) throws
IOException { }
@Override
public Map> get(URI uri, Map>
requestHeaders) throws IOException {
return Collections.emptyMap();
}
});
// Any site which uses cookies (i.e. Set-Cookie or Set-Cookie2 header)
URL url = new URL("https://oracle.com";);
url.openConnection().getHeaderFields();
```
Running this code with `openjdk 15 2020-09-15` shows that the call to
`SharedSecrets.getJavaNetHttpCookieAccess()` is made before the class
`HttpCookie` has
been accessed and initialized. Therefore merely running this code in two
separate threads
(both having been started before the code is executed, since `Thread.start()`
establishes
a happens-before relationship) should be enough to render that `SharedSecrets`
method
non-thread-safe.
Kind regards
[0] https://docs.oracle.com/javase/specs/jls/se15/html/jls-12.html#jls-12.4.2
> Hans Boehm hat am 29. Dezember 2020 um 18:53 geschrieben:
>
> If static_field is not volatile, and set concurrently, then the first read of
> static_field may return non-null and the second null, without initialize()
> even being executed. The Java memory model does not prevent reordering of
> non-volatile reads from the same field (for good reason).
>
> Even if initialize() is executed and performs a volatile read, this reasoning
> doesn't hold. The initial static_field read may be delayed past the volatile
> read inside the conditional and hence, at least theoretically, past the
> second read. Control dependencies don't order reads, either in Java, or in
> modern weakly-ordered architectures with branch prediction. This doesn't
> matter if initialize() sets static_field.
>
> This all assumes that having two threads call initialize() is OK.
>
> Java code with data races is extremely tricky and rarely correct.
>
> Hans
Re: Is SharedSecrets thread-safe?
Hello Peter,
the changes look good, however there might be more to consider:
- `jdk.internal.access.SharedSecrets.getJavaLangRefAccess()`
Might be good to add a comment there or for `java.lang.ref.Reference` that it
is (hopefully?)
initialized during JVM start-up. Similar to the comment for
`AccessibleObject` which states
that it is initialized in "initPhase1".
Currently without special knowledge about JVM start-up, it is not obvious
that this construct
is safe.
- `java.io.Console.cons`
This is pretty brittle. It is currently only thread-safe because the only
accessor is
`System.console()` which has its own synchronization. However, I doubt that
the author was aware
that `Console.cons` on its own is not thread-safe.
In general, why is there lazy initialization twice? Once in
`System.console()` and then in the
accessor for `Console.cons`. In my opinion *only* `java.io.Console` should
have the lazy
initialization logic (with proper synchronization!) and make sure that only
one `Console` instance
exists.
- `jdk.internal.access.JavaIOAccess.charset()`
The implementation of this one is extremely brittle. While it documents that
the `Password` class
calling it will make sure that `System.console()` has been called before, in
that `Password` class
there is not such notice, so it could easily happen that someone breaks this
at a later point.
Additionally the field `Console.cs` it accesses is not `final`, making it
even more brittle.
In my opinion there should be two changes:
1. Change `JavaIOAccess.charset()` -> `charset(Console)`, which then accesses
the charset from that
Console argument.
This enforces that the user of the access already has the Console
initialized and indirectly
establishes the happens-before relationship for `Console.cs`.
2. The Console class should have the following fields `final`:
readLock, writeLock, reader, out, pw, formatter, cs
For `cs` this would merely require introducing a local variable.
In general `sun.security.util.Password.convertToBytes(char[])` is problematic
because it makes
passwords unportable when one OS uses a different terminal encoding than
another.
The cleaner backward compatible solution might be to simply block all
non-ASCII chars (i.e. throw
exception for them)?
This would break for all existing users which used non-ASCII chars or where
their terminal has an
encoding not based on ASCII, but these users might already have different
problems due to the
existing implementation.
- `jdk.internal.access.SharedSecrets.setJavaAWTAccess(JavaAWTAccess)`
Might have to establish a happens-before relationship for
`getJavaAWTAccess()` because caller
appears to expect a `JavaAWTAccess` in case respective class has been loaded.
On a side note here: The implementation of that access appears to contain
redundant code:
https://github.com/openjdk/jdk/blob/8435f0daf2413a4c17578dd288e093fe006b3880/src/java.desktop/share/classes/sun/awt/AppContext.java#L866
The null check there makes no sense because `ecx` is always null at that
point.
- `jdk.internal.access.SharedSecrets.setJavaAWTFontAccess(JavaAWTFontAccess)`
This seems to be rather brittle as well because its callers check whether the
access has already
been initialized.
Additionally this seems to be more complicated than it has to be: It appears
to be simpler to have
`SharedSecrets` initialize the access by initializing only `NumericShaper`
(or `TextAttribute`,
ultimately does not matter which class from that package is used) when
`getJavaAWTFontAccess()` is
called. The performance penalty (if any) is likely negligible.
- `com.sun.jmx.mbeanserver.JavaBeansAccessor`
Since the static initializer of that class is loading `Introspector` (which
sets access object)
anyways it might be saner to have this initialization logic in
`SharedSecrets` instead.
Kind regards
> Peter Levart hat am 31. Dezember 2020 um 11:07
> geschrieben:
>
>
>
> On 12/31/20 2:30 AM, Hans Boehm wrote:
> > It sounds as though this would be correct if
> >
> > if (static_field == null) {
> >initialize();
> > }
> > return static_field;
> > ```
> >
> > were rewritten as
> >
> > Foo my_local_copy = static_field;
> > if (my_copy == null) {
> > initialize();
> > my_local_copy = static_field;
> > }
> > return my_local_copy;
> >
> > That would prevent the redundant read of static_field unless this thread
> > did the initialization, in which case the original null would no longer be
> > visible to the second read.
> >
> > Hans
>
>
> I agree. The initialize() part is triggering some class initialization
> where concurrent attempts do establish a HB edge so the 2nd read of
> static_field after initialize() is ordered properly and can't read null.
>
> I created a JIRA ticket here:
> https://bugs.openjdk.java.net/browse/JDK-8259021
>
> And prepared a PR here: https://github.com/openjdk/jdk/pull/1914
>
>
> Happy
Re: Is SharedSecrets thread-safe?
Hello Peter, feel free to consider these issues one at a time, or not at all, if you don't think that it is worth it. However, please note that I will unsubscribe from this mailing list in the next days again due to the high degree of activity. (Related: https://mail.openjdk.java.net/pipermail/discuss/2020-December/005674.html) Kind regards > Peter Levart hat am 4. Januar 2021 um 22:48 > geschrieben: > > > Hello 99206970363698485155, > > > Thanks for these pointers. I would prefer to untangle those knots one at > a time, if you don't mind, since some of them touch other parts of code > while this change to SharedSecrets is pretty straightforward and localized. > > > Regards, Peter > > > On 12/31/20 6:25 PM, some-java-user-99206970363698485...@vodafonemail.de > wrote: > > Hello Peter, > > the changes look good, however there might be more to consider: > > > > - `jdk.internal.access.SharedSecrets.getJavaLangRefAccess()` > >Might be good to add a comment there or for `java.lang.ref.Reference` > > that it is (hopefully?) > >initialized during JVM start-up. Similar to the comment for > > `AccessibleObject` which states > >that it is initialized in "initPhase1". > >Currently without special knowledge about JVM start-up, it is not > > obvious that this construct > >is safe. > > > > - `java.io.Console.cons` > >This is pretty brittle. It is currently only thread-safe because the > > only accessor is > >`System.console()` which has its own synchronization. However, I doubt > > that the author was aware > >that `Console.cons` on its own is not thread-safe. > >In general, why is there lazy initialization twice? Once in > > `System.console()` and then in the > >accessor for `Console.cons`. In my opinion *only* `java.io.Console` > > should have the lazy > >initialization logic (with proper synchronization!) and make sure that > > only one `Console` instance > >exists. > > > > - `jdk.internal.access.JavaIOAccess.charset()` > >The implementation of this one is extremely brittle. While it documents > > that the `Password` class > >calling it will make sure that `System.console()` has been called > > before, in that `Password` class > >there is not such notice, so it could easily happen that someone breaks > > this at a later point. > >Additionally the field `Console.cs` it accesses is not `final`, making > > it even more brittle. > > > >In my opinion there should be two changes: > >1. Change `JavaIOAccess.charset()` -> `charset(Console)`, which then > > accesses the charset from that > > Console argument. > > This enforces that the user of the access already has the Console > > initialized and indirectly > > establishes the happens-before relationship for `Console.cs`. > >2. The Console class should have the following fields `final`: > > readLock, writeLock, reader, out, pw, formatter, cs > > For `cs` this would merely require introducing a local variable. > > > >In general `sun.security.util.Password.convertToBytes(char[])` is > > problematic because it makes > >passwords unportable when one OS uses a different terminal encoding than > > another. > >The cleaner backward compatible solution might be to simply block all > > non-ASCII chars (i.e. throw > >exception for them)? > >This would break for all existing users which used non-ASCII chars or > > where their terminal has an > >encoding not based on ASCII, but these users might already have > > different problems due to the > >existing implementation. > > > > - `jdk.internal.access.SharedSecrets.setJavaAWTAccess(JavaAWTAccess)` > >Might have to establish a happens-before relationship for > > `getJavaAWTAccess()` because caller > >appears to expect a `JavaAWTAccess` in case respective class has been > > loaded. > > > >On a side note here: The implementation of that access appears to > > contain redundant code: > > > > https://github.com/openjdk/jdk/blob/8435f0daf2413a4c17578dd288e093fe006b3880/src/java.desktop/share/classes/sun/awt/AppContext.java#L866 > >The null check there makes no sense because `ecx` is always null at that > > point. > > > > - > > `jdk.internal.access.SharedSecrets.setJavaAWTFontAccess(JavaAWTFontAccess)` > >This seems to be rather brittle as well because its callers check > > whether the access has already > >been initialized. > >Additionally this seems to be more complicated than it has to be: It > > appears to be simpler to have > >`SharedSecrets` initialize the access by initializing only > > `NumericShaper` (or `TextAttribute`, > >ultimately does not matter which class from that package is used) when > > `getJavaAWTFontAccess()` is > >called. The performance penalty (if any) is likely negligible. > > > > - `com.sun.jmx.mbeanserver.JavaBeansAccessor` > >Since the static initializer of that class is loading `Introspector` >
Re: RFR: 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant
The following probably does not matter much because I am not an OpenJDK contributor, but personally I think throwing an UnsupportedOperationException is reasonable: 1. It is consistent with the other methods which also only work for one specific variant 2. Code which calls UUID.version() on a non-variant 2 UUID is obviously already functionally broken; I don't think maintaining backward compatibility here adds any value Regarding the pull request, I would recommend the following changes: 1. Replace ` ` with a normal space, that should work as well and is easier to read 2. Add a sentence to the method description (and not only to the `@throws` tag), that this method is only meaningful for variant 2 UUIDs, see for example the documentation for `timestamp()` for how this sentence should look like to be consistent: https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/util/UUID.html#timestamp() Kind regards
Re: RFR: 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant
> 1. Replace ` ` with a normal space, that should work as well and is easier > to read Looks like my e-mail client was so kind and replaced the HTML character reference. It should have said: "Replace `& nbsp ;` with a normal space, ..." Additionally, if you want to search for projects using UUID.version() you can use the following CodeQL query: https://lgtm.com/query/283083268427438766/ You can (in addition to the example projects), specify custom projects to scan as well, see https://lgtm.com/help/lgtm/project-lists. Kind regards
Improve `finally` block exception handling
Hello, are there any plans to improve exception handling in combination with `finally` blocks? Currently, when a `finally` block does not complete normally, the original exception is silently discarded (as described in the JLS). This behavior is error-prone, not obvious at all, and has been criticized in the past a lot. An example for this is https://stackoverflow.com/q/48088 and the linked posts there. While javac warns you about this issue when using `-Xlint:finally`, it is only a warning and not even enabled by default. Are there any plans to forbid usage of `break`, `continue`, `yield` and `return` in `finally` blocks? Switch expressions have already set a precedent for this by not allowing to leave the switch expression by something other than a `throw` or `yield` statement, trying to use for example a `return` statement causes a compilation error. Similarly for `throw` and any implicitly thrown exceptions, is there a plan to at least add the original exception as suppressed one to the newly thrown? Of course one could argue that the same applies to `catch` clauses whose body accidentally causes an exception which discards the caught one. However the main difference is that there, only a specific exception type is caught (and discarded), whereas for `finally` exceptions of _any_ type are discarded. It could also be argued that adding suppressed exceptions decreases performance or causes memory leaks, but one the other hand this behavior was explicitly included try-with-resources statements, most likely because the value this adds was considered more important than any performance issues this might cause. Also, it is important to keep in mind that this affects _all_ Throwable types, including VM errors which you really should not catch, and which due to a `finally` block might silently be discarded. Most, if not all, code in which a `finally` does not complete normally does not seem to consider this. This is also not a theoretical problem; this issue exists in multiple open source projects, potentially even in the JDK itself. Often the code can be rewritten by either moving the complete code or parts of it outside of the `finally` block, or by introducing a local variable to hold the result and to return that after the `finally` block. What do you think about this topic? Is backward compatibility a concern? If so, can you provide an example where using such a pattern provides any notable advantages which justify the potential loss of thrown VM errors or exceptions. Kind regards
Re: Improve `finally` block exception handling
Thanks a lot for your feedback! My original message was a bit vague in parts,
sorry for that.
The proposal consists of the following:
1. Forbidding usage of `break`, `continue`, `yield` and `return` in `finally`
blocks
2. Adding exceptions as suppressed exceptions:
If exception E1 led to execution of the `finally` block and the block is
left due to
another exception E2, then either E1 or E2 should be thrown with the other
one added
as suppressed exception. For consistency with try-with-resources and because
E1 is
most likely more relevant ideally E1 would be thrown and E2 should be
suppressed. But
if you think the impact on backward compatibility would be too large, then
E2 should
be thrown (the current behavior), but E1 should at least be added as
suppressed exception.
The following replies to your comments hopefully make the rationale for these
proposed
changes clearer.
> The behaviour of try/catch/finally is not "obvious at all", you have to read
> what it means
> and when you do that you clearly see what happens regarding exceptions.
For try-catch you see that the code catches some specific exception and then
handles it in a
certain way, whether that is by rethrowing, logging or itentionally ignoring it.
The issue with `finally` is that in its current form, exceptions which occurred
in the `try`
block might just silently disappear. Consider this simple example:
```
try {
throw new RuntimeException();
} finally {
return true;
}
```
Here it is not clear at all, unless you have read the JLS in detail, that the
thrown exception
just vanishes. There is no explicit indication that the `finally` has any
effect on the
thrown exception. Of course this is a contrived example, but consider the same
situation where
the `try` block calls a method which might throw an exception (or the general
case that a
VirtualMachineError occurs), and that the `return` (or any of the other
affected statements)
is not the only statement in the `finally` block.
Similar confusing code can be written where the `try` or `catch` block returns
a value
(or continues or breaks loop iteration), but the `finally` block overwrites
that action:
```
try {
return 1;
} finally {
return 2;
}
```
Again, contrived, but consider the same code with additional statements in the
`try` and
`finally` blocks. This breaks fundamental assumptions one has about the
behavior of the
statements, such as `return`.
>> Are there any plans to forbid usage of `break`, `continue`, `yield` and
>> `return` in
>> `finally` blocks?
>
> Why would we do that? What would that gain?
It would prevent code such as the one shown above, where code, most likely
unintentionally,
discards exceptions. Now also consider that inside the `try` block a
VirtualMachineError
(or a subclass of it) may occur which you really should not catch. Yet the code
in the
`finally` block silently discards it without you ever noticing the error,
before it occurs
later in another part of the application again and has possibly already
corrupted the state
of the application.
>> Similarly for `throw` and any implicitly thrown exceptions, is there a plan
>> to at least
>> add the original exception as suppressed one to the newly thrown?
>
> That suggestion is not completely without merit, but nor is it a "slam-dunk"
> obvious thing to do. The semantic implications of the exceptions matter, and
> semantics come from the programmers intent. There's no reasonable way to
> automagically determine that when an exception is created that another
> exception (that led to the finally block) should be inserted as a "suppressed
> exception". That would actually be completely wrong to do in many
> circumstances you would instead need to act when the exception would
> terminate the finally block and then add the original exception as the
> "suppressed" exception.
To clarify my suggestion: If a `finally` block is entered due to an exception
E1, and is
exited due to an exception E2 (regardless of whether explicitly thrown by a
`throw` statement),
and E1 != E2, then both exceptions should be preserved, with one being added as
suppressed
exception to the other one.
> But really the programmer is in the best position to decide how exceptions
> need to be handled.
Except that in a `finally` block they don't have access to the exception which
led to execution
of the `finally` block, unless they write verbose hacky code to first have a
`catch (Throwable)`
which stores the caught exception in a local variable. To me it appears for
many `finally`
blocks in existing code it was not actively decided how exception handling
should be done,
but rather it was forgotten that the original exception might get discarded
when the `finally`
throws a new exception; or that behavior was considered acceptable because
currently Java does
not allow you to handle it in a better way (which is one of the main points of
this proposal).
>> Of course one cou
Feature suggestion: Allow generic wildcard in class literal expression
JDK-6184881 describes that Object.getClass() should ideally not return classes
of raw types anymore but instead use wildcards. The problem is that, as noted
in the comments, this breaks compability with previously written code and
therefore this change is not very likely (at the moment?).
However, the report does not cover the class literal expression
(https://docs.oracle.com/javase/specs/jls/se11/html/jls-15.html#jls-15.8.2).
It would be good to allow the usage of the wildcard (`?`) for this expression:
List.class => Class>
Since it is currently not allowed to provide type parameters in the class
literal expression there should not be any compatibility issues.
Additionally if at some point JDK-6184881 is fixed for class literals as well,
then the expression List.class could just be a verbose variant of List.class
without causing any problems either.
This would make writing methods or constructors which determine a generic type
based on a given class easier for generic classes, e.g.:
public class MyContainer {
private final Class valueClass;
public MyContainer(Class valueClass) {
this.valueClass = valueClass;
}
public static void main(String[] args) {
MyContainer stringContainer = new MyContainer<>(String.class);
// Have to use raw type (or verbose casting of class)
MyContainer listContainer = new MyContainer<>(List.class);
// With suggested change:
MyContainer> listContainer = new MyContainer<>(List.class);
}
}
Feature suggestion: Add static equals methods to Float and Double
To test whether primitive float or double values are equal according to
`Float.equals` and `Double.equals` you either have to create wrapper instances
for them (possible performance decrease), use the respective static `compareTo`
(verbose) or have to use the appropriate methods (`floatToIntBits` /
`doubleToLongBits`) (verbose and error-prone since you could confuse them with
the other conversion methods).
It would be good to provide static methods for testing for equality of the
primitive values:
// In Float.java
public static boolean equals(float a, float b) {
return Float.floatToIntBits(a) == Float.floatToIntBits(b);
}
// In Double.java
public static boolean equals(double a, double b) {
return Double.doubleToLongBits(a) == Double.doubleToLongBits(b);
}
This would be very convenient for developers and prevent them from writing
(possibly faulty) code for this themselves.
Feature suggestion: Provide efficient way for nullable value lookup in Map
The methods currently provided by the Map interface
(https://docs.oracle.com/javase/8/docs/api/java/util/Map.html) do not provide
an efficient way to look up nullable values. This problem would be solved if
JDK-6552529 was implemented, but that will likely not be happening since the
interface cannot be changed without breaking compatibility.
Ways to currently look up nullable values are:
* Combined ussage of `get()` and `containsKey()` -> inefficient
* Usage of `getOrDefault()` with private no-value constant, e.g.:
private static final Object NO_VALUE = new Object();
// ...
V value = map.getOrDefault(key, (V) NO_VALUE);
if (value == NO_VALUE) {
// No value
}
else {
// Value exists
}
Both solutions are not really that great, therefore it would be good to provide
an easier and (if overridden) more efficient method: default
OptionalNullable getOptional(Object key) {
V value = get(key);
if (value != null || containsKey(key)) {
return OptionalNullable.of(value);
}
else {
return OptionalNullable.empty();
}
}
Where `OptionalNullable` is a new class similar to `Optional` except that null
is considered a value.
Maybe a `void ifExists(K key, Consumer consumer)`, which makes the consumer
consume the value if it exists (= nullable), would be good as well to avoid
creation of `OptionalNullable` objects, though that method might (at least for
performance reasons) become obsolute when `OptionalNullable` becomes a value
type.
Re: Feature suggestion: Add static equals methods to Float and Double
Hello Remi,
You are right, the proposed method names would prevent backwards compatibility,
I forgot to think
about that. Sorry for the trouble.
Maybe a different, currently not used name, such as `areEqual`,
`primitiveEquals` or similar would
be better.
What do You think about the addition of such a method?
Kind regards
> Remi Forax hat am 5. Januar 2019 um 00:50 geschrieben:
>
>
> Hi,
> it's not obvious to me that this is a source backward compatible change.
>
> if you have something like:
> interface Fun { boolean eq(Double a, double b); }
> ...
> Fun fun = Double::equals;
>
> it's not clear to me which variant will be selected ?
>
> regards,
> Rémi
Re: Feature suggestion: Add static equals methods to Float and Double
If the developer implemented the Comparable interface correctly, the method you
proposed would be
equivalent to java.util.Objects.equals(Object, Object).
Additionally both variants would require boxing for primitive types which I
initially wanted to prevent.
> Zheka Kozlov hat am 6. Januar 2019 um 11:56
> geschrieben:
>
>
> Why don't we just add a generic method which compares any instances of
> Comparable?
>
>
> public interface Comparator {
> ...
>
> public static > boolean equal(T x, T y) {
> return x.compareTo(y) == 0;
> }
> }
>
>
> Usage:
>
> Comparator.equal(1, 1); // true
> Comparator.equal(1.0, 1.0); // true
> Comparator.equal(2.0f, 2.0f); // true
> Comparator.equal("abc", "def"); // false
> Comparator.equal("abc", 1); // compilation error
Re: Feature suggestion: Add static equals methods to Float and Double
My main goal was to provide a way where NaN is equal to NaN and I went with the behavior of the respective wrapper classes, Float and Double, which consider -0.0 and +0.0 as not equal. The static method could be called "exactlyEquals" if that is better. It might also depend on the usecase whether you want -0.0 being equal to +0.0. Maybe an additional method would be required for the case you are describing, e.g. "valueEquals" (though that can be ambiguous). > Hans Boehm hat am 6. Januar 2019 um 19:40 geschrieben: > > > What's the motivation for making it easier to spell a comparison method that > tells me that -1.0*0.0 is not equal to 1.0*0.0? I would have thought that > making this somewhat hard to write is a feature? > > I agree that there are cases in which this method is useful. But they all > seem esoteric enough to me that it's not unreasonable for users to have > enough expertise to write the method. Or at least the method should have a > name that makes the unusual behavior explicit.
Re: Feature suggestion: Provide efficient way for nullable value lookup in Map
Yes it is now possible to implement the methods `getKey(Object key)` and `getEntry(Object key)` requested by JDK-6552529 as default methods, however both would have to include that "The default implementation returns the provided key. Overriding implementations may return the actual key used by the map." In this case it is questionable how useful the methods actually are, e.g. Two key objects k1 and k2 with `k1.equals(k2)` Two objects m1 and m2 of different map implementations both containing k1. Then the following is possible: m1.getKey(k2) == k2 // true m2.getKey(k2) == k2 // false > Brian Goetz hat am 7. Januar 2019 um 00:54 > geschrieben: > > > FYI, the comment about compatibility was obsoleted by the addition of > default methods in Java 8.
Re: Feature suggestion: Provide efficient way for nullable value lookup in Map
Would this method then be useful enough if the default implementation is that
inefficient (in case
you only want to get a nullable value)?
> Brian Goetz hat am 8. Januar 2019 um 16:57
> geschrieben:
>
>
> Here's a default implementation that returns the actual key:
>
> default Optional> getEntry(K key) {
> for (Map.Entry e : entrySet) {
> if (Objects.equals(key, e.getKey())
> return Optional.of(e);
> }
> return Optional.empty();
> }
>
> On 1/8/2019 10:50 AM,
> some-java-user-99206970363698485...@vodafonemail.de wrote:
> > Yes it is now possible to implement the methods `getKey(Object key)` and
> > `getEntry(Object key)`
> > requested by JDK-6552529 as default methods, however both would have to
> > include that
> > "The default implementation returns the provided key. Overriding
> > implementations may return the
> > actual key used by the map."
> > In this case it is questionable how useful the methods actually are, e.g.
> >Two key objects k1 and k2 with `k1.equals(k2)`
> >Two objects m1 and m2 of different map implementations both containing
> > k1.
> >Then the following is possible:
> >m1.getKey(k2) == k2 // true
> >m2.getKey(k2) == k2 // false
> >
> >> Brian Goetz hat am 7. Januar 2019 um 00:54
> >> geschrieben:
> >>
> >>
> >> FYI, the comment about compatibility was obsoleted by the addition of
> >> default methods in Java 8.
>
Re: Feature suggestion: Allow generic wildcard in class literal expression
What do you think about this idea? I hope the fact that I did not get any
responses yet does not mean that you are not interested or that I annoyed you.
> JDK-6184881 describes that Object.getClass() should ideally not return
> classes of raw types anymore but instead use wildcards. The problem is that,
> as noted in the comments, this breaks compability with previously written
> code and therefore this change is not very likely (at the moment?).
>
> However, the report does not cover the class literal expression
> (https://docs.oracle.com/javase/specs/jls/se11/html/jls-15.html#jls-15.8.2).
>
> It would be good to allow the usage of the wildcard (`?`) for this
> expression:
> List.class => Class>
>
> Since it is currently not allowed to provide type parameters in the class
> literal expression there should not be any compatibility issues.
> Additionally if at some point JDK-6184881 is fixed for class literals as
> well, then the expression List.class could just be a verbose variant of
> List.class without causing any problems either.
>
>
> This would make writing methods or constructors which determine a generic
> type based on a given class easier for generic classes, e.g.:
>
> public class MyContainer {
> private final Class valueClass;
>
> public MyContainer(Class valueClass) {
> this.valueClass = valueClass;
> }
>
> public static void main(String[] args) {
> MyContainer stringContainer = new
> MyContainer<>(String.class);
> // Have to use raw type (or verbose casting of class)
> MyContainer listContainer = new MyContainer<>(List.class);
>
> // With suggested change:
> MyContainer> listContainer = new
> MyContainer<>(List.class);
> }
> }
>
