Fix socket write NPE and avoid throwaway allocation on partial writes (#5139)#5140
Merged
Conversation
…#5139) Two issues reported in #5139 against the iOS socket code: 1. NPE instead of IOException: a write reaching a torn-down native socket handle surfaced as a NullPointerException deep inside the platform implementation (((Long)socket).longValue() on a null handle) rather than an IOException the caller can handle. SocketOutputStream now routes every write overload through a single guarded path that throws IOException when the socket handle is null, and validates the offset/length arguments. 2. Throwaway allocation on partial writes: write(b, off, len) with a non-zero offset allocated a fresh byte[] + System.arraycopy on every call just to pass the data down. Added an offset-aware writeToSocketStream(socket, data, offset, len) to the implementation API. The base implementation keeps the copy as a backward-compatible default; the iOS (native sub-range via arrayToDataRange), Android and JavaSE ports override it to write the sub-range directly with no intermediate copy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 11 screenshots: 11 matched. |
Contributor
Cloudflare Preview
|
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Collaborator
Author
|
Compared 122 screenshots: 122 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 122 screenshots: 122 matched. Benchmark Results
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 122 screenshots: 122 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 122 screenshots: 122 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the two problems raised in #5139 about the socket write path.
1. NPE instead of IOException
The reported crash was a
NullPointerExceptionthrown deep in the platform implementation:The only Java-level null dereference in that path is
((Long)socket).longValue()inIOSImplementation.writeToSocketStreamwhen the socket handle is null — i.e. a writer thread reached a socket whose native handle was already torn down. That surfaced as an opaque NPE rather than anIOExceptionthe caller could catch.SocketOutputStreamnow routes all threewrite(...)overloads through a single guarded helper that throwsIOException("Socket is not connected")when the handle is null, and validates theoffset/lenarguments ofwrite(byte[], int, int).2. Throwaway allocation on partial writes
As noted in the issue,
write(b, off, len)with a non-zero offset allocated a freshbyte[]andSystem.arraycopy'd into it on every call, purely to hand a whole-array view to the implementation.This adds an offset-aware
writeToSocketStream(Object socket, byte[] data, int offset, int len)to the implementation API:CodenameOneImplementation— default implementation keeps the copy (backward compatible for any port/lib that doesn't override it).offset/lendown to a new native binding; the C side builds a sub-rangeNSDatavia a newarrayToDataRangehelper, so no intermediate Java array is allocated.OutputStream.write(data, offset, len)directly.Notes
IOSNative.writeToSocketStream(long, byte[], int, int)); the iOS port must be rebuilt.corecompiles cleanly with JDK 8.JavaSEPort.javacompiles (the only errors in a localjavasebuild are pre-existing JavaFX-not-on-classpath issues in the unrelatedfx/package).🤖 Generated with Claude Code