Add GsonBuilder.disableJdkUnsafe()
#1904
Conversation
Marcono1234
force-pushed
the
marcono1234/disable-unsafe
branch
from
3f67886 to
802d85f
Compare
Marcono1234
force-pushed
the
marcono1234/disable-unsafe
branch
2 times, most recently
from
21d0f85 to
2b70a54
Compare
Marcono1234
force-pushed
the
marcono1234/disable-unsafe
branch
from
2b70a54 to
bb5adf3
Compare
|
Maybe in the future it should also be considered to make usage of |
eamonnmcmanus
left a comment
eamonnmcmanus
left a comment
There was a problem hiding this comment.
Thanks, this is great! I think using Unsafe.allocateInstance is kind of horrifying, and it certainly shouldn't be the default behaviour. Unfortunately, it is, and changing that would probably break too many things. Making it at least possible to avoid Unsafe is probably the best we can do.
| * runtimes. For example Android does not provide {@code Unsafe}, or only with limited | ||
| * functionality. Additionally {@code Unsafe} creates instances without executing any | ||
| * constructor or initializer block, or performing initialization of field values. This can | ||
| * lead to surprising and difficult to debug errors.<br> |
There was a problem hiding this comment.
Could you start a new paragraph on the next line rather than <br>? Or just leave out <br>.
| if (useJdkUnsafe) { | ||
| return new ObjectConstructor<T>() { | ||
| private final UnsafeAllocator unsafeAllocator = UnsafeAllocator.create(); | ||
| @SuppressWarnings("unchecked") |
There was a problem hiding this comment.
While you are here, could you reduce the scope of this?
@Override public T construct() {
...
@SuppressWarnings("unchecked")
T newInstance = (T) unsafeAllocator.newInstance(rawType);
return newInstance;
...
|
|
||
| public void testDisableJdkUnsafe() { | ||
| Gson gson = new GsonBuilder() | ||
| .disableJdkUnsafe() |
There was a problem hiding this comment.
Very nitpicky comment here, but these continuation lines should be indented +4 according to Google Java style.
There was a problem hiding this comment.
No worries, such comments are fine
2 participants
Resolves #445
Resolves partially #1387
Resolves partially #1695; should still be kept open because usage of
Unsafeshould be better documentedNo test is added to verify that by default
Unsafeis used because it appears 38 tests already depend on this behavior and would fail that was not the case.Note that for Android this actually disables more than just
sun.misc.Unsafeusage, because there Gson also tries internalObjectStreamClassandObjectInputStreammethods, but I don't think this is worth mentioning.