From 122f8bb7b35c38800c37bea8de9c8db19fb0e54f Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 26 Jul 2026 07:33:27 +0300 Subject: [PATCH 01/24] core: loopback server sockets, so MCP can serve any platform MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MCP's socket transport was JavaSE-only, and for a real reason: it binds a server socket, and the portable com.codename1.io.Socket API only binds the wildcard address — which would publish a channel that can read the screen and drive the UI on every network interface. So attaching an agent to an app running on a device was simply not possible. This adds the missing capability rather than working around it: - Socket.listenLoopback(port, class) and isLoopbackServerSocketSupported(), backed by a new CodenameOneImplementation.listenSocketLoopback hook. It is deliberately SEPARATE from listenSocket and defaults to unsupported: a port must implement loopback binding explicitly, because answering this with the wildcard implementation would silently widen the reach of a channel the caller asked to keep local. There is no fallback for the same reason. - MCPLoopbackSocketTransport in core, built on that API, used automatically by any port that did not register a transport of its own. JavaSE keeps its java.net implementation. - JavaSE and Android bind InetAddress.getLoopbackAddress(). Their server-socket caches key loopback and wildcard sockets separately, so a port already bound wildcard can never be handed to a caller that asked for loopback. - iOS gains server sockets at all: it had none (listenSocket returned null). The native implementation binds INADDR_LOOPBACK, listens, and wraps each accepted descriptor in a CFStream pair, yielding the VM thread across the blocking accept. Wildcard listening stays unsupported there, so an iOS app cannot accidentally publish a port. Tests cover the framing and session handling: frame delimiting independent of packet boundaries, CRLF tolerance, write-with-no-client, reconnect without ending the server, and close waking a pending read. Co-Authored-By: Claude Opus 5 (1M context) --- .../impl/CodenameOneImplementation.java | 29 ++ CodenameOne/src/com/codename1/io/Socket.java | 49 +++- CodenameOne/src/com/codename1/mcp/MCP.java | 21 +- .../mcp/MCPLoopbackSocketTransport.java | 259 ++++++++++++++++++ .../impl/android/AndroidImplementation.java | 45 ++- .../com/codename1/impl/javase/JavaSEPort.java | 47 +++- Ports/iOSPort/nativeSources/IOSNative.m | 15 + Ports/iOSPort/nativeSources/SocketImpl.h | 1 + Ports/iOSPort/nativeSources/SocketImpl.m | 107 ++++++++ .../codename1/impl/ios/IOSImplementation.java | 16 ++ .../src/com/codename1/impl/ios/IOSNative.java | 1 + .../mcp/MCPLoopbackSocketTransportTest.java | 171 ++++++++++++ .../TestCodenameOneImplementation.java | 14 + 13 files changed, 744 insertions(+), 31 deletions(-) create mode 100644 CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java create mode 100644 maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java diff --git a/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java b/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java index 329c97724ee..e790b48d89b 100644 --- a/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java +++ b/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java @@ -9917,6 +9917,35 @@ public Object listenSocket(int port) { throw new RuntimeException("Not supported"); } + /// Whether this port can bind a server socket to the LOOPBACK interface only. + /// + /// Deliberately separate from [#isServerSocketAvailable()] and defaulting to false: + /// a port must implement loopback binding explicitly. Answering this with the + /// wildcard-binding implementation would publish on every network interface a + /// channel the caller asked to keep local. + /// + /// #### Returns + /// + /// true if [#listenSocketLoopback(int)] is implemented here + public boolean isLoopbackServerSocketAvailable() { + return false; + } + + /// Listens on the given port, bound to the loopback interface only, and blocks until + /// a connection arrives — the accept semantics of [#listenSocket(int)], with a + /// narrower bind. + /// + /// #### Parameters + /// + /// - `port`: the port to listen on + /// + /// #### Returns + /// + /// connected socket instance, or null when the accept failed + public Object listenSocketLoopback(int port) { + throw new RuntimeException("Loopback server sockets are not supported on this platform"); + } + /// Returns the device host or ip address if available /// /// #### Returns diff --git a/CodenameOne/src/com/codename1/io/Socket.java b/CodenameOne/src/com/codename1/io/Socket.java index 88a3b3411fe..1c7fb6f66c2 100644 --- a/CodenameOne/src/com/codename1/io/Socket.java +++ b/CodenameOne/src/com/codename1/io/Socket.java @@ -61,6 +61,22 @@ public static boolean isServerSocketSupported() { return Util.getImplementation().isServerSocketAvailable(); } + /// Returns true if this port can listen on the LOOPBACK interface only, so the + /// channel is reachable from the device itself but not from the network. + /// + /// This is a strictly narrower capability than [#isServerSocketSupported()], which + /// binds the wildcard address and therefore publishes the port on every network + /// interface. A port must implement loopback binding explicitly; there is no + /// fallback to a wildcard bind, because silently widening the reach of a channel a + /// caller asked to keep local would be the wrong failure. + /// + /// #### Returns + /// + /// true if [#listenLoopback(int, Class)] is usable on this port + public static boolean isLoopbackServerSocketSupported() { + return Util.getImplementation().isLoopbackServerSocketAvailable(); + } + /// Connect to a remote host /// /// #### Parameters @@ -166,6 +182,35 @@ public void close() throws IOException { /// @deprecated server sockets are only supported on Android and Desktop and as such /// we recommend against using them public static StopListening listen(final int port, final Class scClass) { + return listenImpl(port, scClass, false); + } + + /// Listens on the given port on the LOOPBACK interface only, so the channel is + /// reachable from this device but not from the network. Otherwise identical to + /// [#listen(int, Class)]: `scClass` is instantiated per incoming connection and + /// must have a public no-argument constructor. + /// + /// Use this for anything that is local by nature — a debug or automation channel, + /// an on-device tool talking to a companion process. Callers that genuinely want to + /// serve the network should use [#listen(int, Class)] and say so. + /// + /// Fails (never falls back to a wildcard bind) when + /// [#isLoopbackServerSocketSupported()] is false. + /// + /// #### Parameters + /// + /// - `port`: the device port + /// + /// - `scClass`: class of callback for each incoming connection + /// + /// #### Returns + /// + /// StopListening instance that allows the caller to stop listening + public static StopListening listenLoopback(final int port, final Class scClass) { + return listenImpl(port, scClass, true); + } + + private static StopListening listenImpl(final int port, final Class scClass, final boolean loopbackOnly) { class Listener implements StopListening, Runnable { private boolean stopped; @@ -173,7 +218,9 @@ class Listener implements StopListening, Runnable { public void run() { try { while (!stopped) { - final Object connection = Util.getImplementation().listenSocket(port); + final Object connection = loopbackOnly + ? Util.getImplementation().listenSocketLoopback(port) + : Util.getImplementation().listenSocket(port); final SocketConnection sc = (SocketConnection) scClass.newInstance(); if (connection != null) { sc.setConnected(true); diff --git a/CodenameOne/src/com/codename1/mcp/MCP.java b/CodenameOne/src/com/codename1/mcp/MCP.java index be6800e08a8..7c5db2d46ea 100644 --- a/CodenameOne/src/com/codename1/mcp/MCP.java +++ b/CodenameOne/src/com/codename1/mcp/MCP.java @@ -78,14 +78,24 @@ public static void setSocketTransportFactory(SocketTransportFactory factory) { socketTransportFactory = factory; } + /// Whether the PORT supplied a transport of its own. Distinct from + /// [#isSocketSupported()], which also counts the portable fallback — the fallback + /// must not see itself as already installed. + static boolean hasPlatformSocketTransport() { + return socketTransportFactory != null; + } + /// Whether an stdio transport is available on this platform. public static boolean isStdioSupported() { return stdioTransportFactory != null; } - /// Whether a loopback socket transport is available on this platform. + /// Whether a loopback socket transport is available on this platform — either one the + /// port registered itself, or the portable one, which needs + /// [com.codename1.io.Socket#isLoopbackServerSocketSupported()]. public static boolean isSocketSupported() { - return socketTransportFactory != null; + return socketTransportFactory != null + || com.codename1.io.Socket.isLoopbackServerSocketSupported(); } /// Returns the shared server, creating it on first use. @@ -116,9 +126,14 @@ public static synchronized MCPServer startStdioServer() { /// Starts a loopback socket server so an agent can attach to this running process. /// Requires a platform socket transport factory (registered by the JavaSE port). public static synchronized MCPServer startSocketServer(int port) { + // Ports that did not register a transport of their own fall back to the portable + // one, which binds loopback through com.codename1.io.Socket — so attaching to a + // running app on a device or simulator works, not only on the desktop. + MCPLoopbackSocketTransport.registerIfPlatformHasNone(); if (socketTransportFactory == null) { throw new IllegalStateException( - "No socket MCP transport is available on this platform. Run on the JavaSE port."); + "No socket MCP transport is available on this platform: it cannot bind a " + + "loopback server socket."); } MCPServer s = getServer(); s.start(socketTransportFactory.createSocketTransport(port)); diff --git a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java new file mode 100644 index 00000000000..d54444491e9 --- /dev/null +++ b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java @@ -0,0 +1,259 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.codename1.mcp; + +import com.codename1.io.Socket; +import com.codename1.io.SocketConnection; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; + +/// Loopback socket MCP transport built on the portable [com.codename1.io.Socket] API, so +/// an agent can attach to a running app on ANY port that can bind the loopback interface — +/// a device or simulator, not only the desktop. +/// +/// The channel is bound to loopback only. That is the security boundary: an attaching +/// agent can read the screen and drive the UI, so it must already be on the device (or on +/// a host forwarding into it), never merely on the same network. There is no wildcard +/// fallback; if the port cannot bind loopback this transport refuses to open. +/// +/// The listening socket stays bound across client sessions, so an agent may disconnect and +/// reconnect — each drive call is typically its own short-lived connection — without the +/// server thread exiting. +public final class MCPLoopbackSocketTransport implements MCPTransport { + /// Lets [Connection], which the socket API instantiates reflectively via a public + /// no-argument constructor, find the transport it belongs to. A single reference + /// suffices because MCP runs one server per process. + private static MCPLoopbackSocketTransport active; + + private final int port; + private final Object lock = new Object(); + private Socket.StopListening listening; + private InputStream in; + private OutputStream out; + private boolean closed; + + MCPLoopbackSocketTransport(int port) { + this.port = port; + } + + public int getPort() { + return port; + } + + /// Registers this transport as the socket factory when the platform has not supplied + /// one of its own. The JavaSE port keeps its native implementation (it can use + /// java.net directly); every other port with loopback support gets this one. + static void registerIfPlatformHasNone() { + if (MCP.hasPlatformSocketTransport()) { + return; + } + MCP.setSocketTransportFactory(new MCP.SocketTransportFactory() { + @Override + public MCPTransport createSocketTransport(int port) { + return new MCPLoopbackSocketTransport(port); + } + }); + } + + @Override + public void open() throws IOException { + if (!Socket.isLoopbackServerSocketSupported()) { + throw new IOException("This platform cannot bind a loopback server socket, so an " + + "MCP agent cannot attach to it"); + } + synchronized (MCPLoopbackSocketTransport.class) { + if (active != null && active != this) { + throw new IOException("Another MCP socket transport is already open on port " + + active.port); + } + active = this; + } + listening = Socket.listenLoopback(port, Connection.class); + } + + /// Called from the connection callback thread when a client attaches. The previous + /// client, if any, is dropped: one agent drives at a time. + void attach(InputStream is, OutputStream os) { + synchronized (lock) { + in = is; + out = os; + lock.notifyAll(); + } + } + + /// Called when a client's session ends. + void detach(InputStream is) { + synchronized (lock) { + if (in == is) { + in = null; + out = null; + lock.notifyAll(); + } + } + } + + @Override + public String readMessage() throws IOException { + while (true) { + InputStream stream; + synchronized (lock) { + while (in == null && !closed) { + try { + lock.wait(); + } catch (InterruptedException ex) { + // fall through: re-check the loop conditions + } + } + if (closed) { + return null; + } + stream = in; + } + String line = readLine(stream); + if (line != null) { + return line; + } + // client disconnected; wait for the next one rather than ending the server + detach(stream); + if (isClosed()) { + return null; + } + } + } + + /// Reads one newline-delimited UTF-8 message. Returns null at end of stream. + /// + /// Read byte at a time rather than through a buffered reader: a buffer would swallow + /// bytes belonging to the next message, and this transport hands the same stream back + /// after a reconnect. + private String readLine(InputStream stream) throws IOException { + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + while (true) { + int b; + try { + b = stream.read(); + } catch (IOException ex) { + return null; // a read error on the client is a disconnect + } + if (b < 0) { + return buffer.size() == 0 ? null : toUtf8(buffer); + } + if (b == '\n') { + return toUtf8(buffer); + } + if (b != '\r') { + buffer.write(b); + } + } + } + + private static String toUtf8(ByteArrayOutputStream buffer) throws IOException { + return new String(buffer.toByteArray(), "UTF-8"); + } + + @Override + public void writeMessage(String message) throws IOException { + OutputStream stream; + synchronized (lock) { + stream = out; + } + if (stream == null) { + throw new IOException("No MCP client is attached"); + } + stream.write(message.getBytes("UTF-8")); + stream.write('\n'); + stream.flush(); + } + + private boolean isClosed() { + synchronized (lock) { + return closed; + } + } + + @Override + public void close() { + Socket.StopListening l; + InputStream is; + synchronized (lock) { + closed = true; + l = listening; + listening = null; + is = in; + in = null; + out = null; + lock.notifyAll(); + } + synchronized (MCPLoopbackSocketTransport.class) { + if (active == this) { + active = null; + } + } + if (l != null) { + l.stop(); + } + if (is != null) { + try { + is.close(); + } catch (IOException ignored) { + // best effort: closing is what unblocks a pending read + } + } + } + + /// The per-connection callback the socket API instantiates. Public with a no-argument + /// constructor because [Socket#listenLoopback] creates it reflectively. + public static final class Connection extends SocketConnection { + @Override + public void connectionError(int errorCode, String message) { + // The listen loop reports its own failures; nothing useful to add per client. + } + + @Override + public void connectionEstablished(InputStream is, OutputStream os) { + MCPLoopbackSocketTransport transport; + synchronized (MCPLoopbackSocketTransport.class) { + transport = active; + } + if (transport == null) { + return; + } + transport.attach(is, os); + // Hold this callback thread for the life of the session: the socket API closes + // the streams as soon as it returns, and the server reads them from its own + // thread. + synchronized (transport.lock) { + while (transport.in == is && !transport.closed) { + try { + transport.lock.wait(); + } catch (InterruptedException ex) { + break; + } + } + } + } + } +} diff --git a/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java b/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java index be652ba5258..e0d87bf75b2 100644 --- a/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java +++ b/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java @@ -11758,18 +11758,25 @@ class ServerSockets { Map socks = new HashMap(); public synchronized ServerSocket get(int port) throws IOException { - if (socks.containsKey(port)) { - ServerSocket sock = socks.get(port); - if (sock.isClosed()) { - sock = new ServerSocket(port); - socks.put(port, sock); - } - return sock; - } else { - ServerSocket sock = new ServerSocket(port); - socks.put(port, sock); - return sock; + return get(port, false); + } + + /** + * When loopbackOnly is set the socket binds 127.0.0.1 rather than the wildcard + * address, so the channel isn't published on every network interface. The two + * are cached separately: a port that is already bound to the wildcard address + * must never be handed back to a caller that asked for loopback. + */ + public synchronized ServerSocket get(int port, boolean loopbackOnly) throws IOException { + Integer key = Integer.valueOf(loopbackOnly ? -port : port); + ServerSocket sock = socks.get(key); + if (sock == null || sock.isClosed()) { + sock = loopbackOnly + ? new ServerSocket(port, 50, InetAddress.getLoopbackAddress()) + : new ServerSocket(port); + socks.put(key, sock); } + return sock; } @@ -11897,8 +11904,12 @@ public void disconnect() { } public Object listen(int param) { + return listen(param, false); + } + + public Object listen(int param, boolean loopbackOnly) { try { - ServerSocket serverSocketInstance = getServerSockets().get(param); + ServerSocket serverSocketInstance = getServerSockets().get(param, loopbackOnly); socketInstance = serverSocketInstance.accept(); SocketImpl si = new SocketImpl(); si.socketInstance = socketInstance; @@ -11940,6 +11951,16 @@ public Object listenSocket(int port) { return new SocketImpl().listen(port); } + @Override + public boolean isLoopbackServerSocketAvailable() { + return true; + } + + @Override + public Object listenSocketLoopback(int port) { + return new SocketImpl().listen(port, true); + } + @Override public String getHostOrIP() { try { diff --git a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java index 32820be36e2..5512dc3ac71 100644 --- a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java +++ b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java @@ -17122,23 +17122,26 @@ private synchronized ServerSockets getServerSockets() { class ServerSockets { Map socks = new HashMap(); - + public synchronized ServerSocket get(int port) throws IOException { - if (socks.containsKey(port)) { - ServerSocket sock = socks.get(port); - if (sock.isClosed()) { - sock = new ServerSocket(port); - socks.put(port, sock); - } - return sock; - } else { - ServerSocket sock = new ServerSocket(port); - socks.put(port, sock); - return sock; + return get(port, false); + } + + /// When loopbackOnly is set the socket binds 127.0.0.1 instead of the wildcard, so + /// the channel is not published on every network interface. The two are cached + /// separately: a port already bound wildcard must never be handed back to a caller + /// that asked for loopback. + public synchronized ServerSocket get(int port, boolean loopbackOnly) throws IOException { + Integer key = Integer.valueOf(loopbackOnly ? -port : port); + ServerSocket sock = socks.get(key); + if (sock == null || sock.isClosed()) { + sock = loopbackOnly + ? new ServerSocket(port, 50, InetAddress.getLoopbackAddress()) + : new ServerSocket(port); + socks.put(key, sock); } + return sock; } - - } class SocketImpl { @@ -17269,8 +17272,12 @@ public void disconnect() { } public Object listen(int param) { + return listen(param, false); + } + + public Object listen(int param, boolean loopbackOnly) { try { - ServerSocket serverSocketInstance = getServerSockets().get(param); + ServerSocket serverSocketInstance = getServerSockets().get(param, loopbackOnly); socketInstance = serverSocketInstance.accept(); return this; } catch(Exception err) { @@ -17303,6 +17310,16 @@ public Object connectSocket(String host, int port, int connectTimeout) { public Object listenSocket(int port) { return new SocketImpl().listen(port); } + + @Override + public boolean isLoopbackServerSocketAvailable() { + return true; + } + + @Override + public Object listenSocketLoopback(int port) { + return new SocketImpl().listen(port, true); + } @Override public String getHostOrIP() { diff --git a/Ports/iOSPort/nativeSources/IOSNative.m b/Ports/iOSPort/nativeSources/IOSNative.m index fdbb4cc31d5..7ced37af245 100644 --- a/Ports/iOSPort/nativeSources/IOSNative.m +++ b/Ports/iOSPort/nativeSources/IOSNative.m @@ -10845,6 +10845,17 @@ JAVA_LONG com_codename1_impl_ios_IOSNative_connectSocket___java_lang_String_int_ return (JAVA_LONG)JAVA_NULL; } +JAVA_LONG com_codename1_impl_ios_IOSNative_listenSocketLoopback___int(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject, JAVA_INT port) { + POOL_BEGIN(); + SocketImpl* impl = [[SocketImpl alloc] init]; + BOOL b = [impl listenLoopback:port]; + POOL_END(); + if(b) { + return (JAVA_LONG)impl; + } + return (JAVA_LONG)JAVA_NULL; +} + JAVA_OBJECT com_codename1_impl_ios_IOSNative_getHostOrIP__(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject) { POOL_BEGIN(); JAVA_OBJECT o = fromNSString(CN1_THREAD_STATE_PASS_ARG [SocketImpl getIP]); @@ -12433,6 +12444,10 @@ JAVA_LONG com_codename1_impl_ios_IOSNative_connectSocket___java_lang_String_int_ return com_codename1_impl_ios_IOSNative_connectSocket___java_lang_String_int_int(CN1_THREAD_STATE_PASS_ARG instanceObject, host, port, connectTimeout); } +JAVA_LONG com_codename1_impl_ios_IOSNative_listenSocketLoopback___int_R_long(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject, JAVA_INT port) { + return com_codename1_impl_ios_IOSNative_listenSocketLoopback___int(CN1_THREAD_STATE_PASS_ARG instanceObject, port); +} + JAVA_OBJECT com_codename1_impl_ios_IOSNative_getHostOrIP___R_java_lang_String(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject) { return com_codename1_impl_ios_IOSNative_getHostOrIP__(CN1_THREAD_STATE_PASS_ARG instanceObject); } diff --git a/Ports/iOSPort/nativeSources/SocketImpl.h b/Ports/iOSPort/nativeSources/SocketImpl.h index 8ad79de9bc3..7e81f7f0a66 100644 --- a/Ports/iOSPort/nativeSources/SocketImpl.h +++ b/Ports/iOSPort/nativeSources/SocketImpl.h @@ -17,6 +17,7 @@ -(void)writeToStream:(NSData*)param; -(void)disconnect; -(BOOL)listen:(int)param; +-(BOOL)listenLoopback:(int)param; -(BOOL)isConnected; -(int)getErrorCode; -(BOOL)isSupported; diff --git a/Ports/iOSPort/nativeSources/SocketImpl.m b/Ports/iOSPort/nativeSources/SocketImpl.m index 622c24a8fa0..2f99b4fc986 100644 --- a/Ports/iOSPort/nativeSources/SocketImpl.m +++ b/Ports/iOSPort/nativeSources/SocketImpl.m @@ -1,5 +1,8 @@ #import "SocketImpl.h" #include +#include +#include +#include #include #include "xmlvm.h" #import "CodenameOne_GLViewController.h" @@ -152,9 +155,113 @@ -(void)disconnect{ } -(BOOL)listen:(int)param{ + // Wildcard-bound server sockets remain unsupported on iOS; only the loopback + // variant below is offered, so an app cannot accidentally publish a listening + // port to the network. return NO; } +/// Listening sockets bound to loopback, keyed by port, kept across accepts so a caller +/// can accept a sequence of connections the way the desktop and Android ports do. +static NSMutableDictionary* _cn1LoopbackListeners = nil; + ++(int)loopbackListenerForPort:(int)port errorOut:(NSString**)errorOut { + @synchronized([SocketImpl class]) { + if(_cn1LoopbackListeners == nil) { + _cn1LoopbackListeners = [[NSMutableDictionary alloc] init]; + } + NSNumber* key = [NSNumber numberWithInt:port]; + NSNumber* existing = [_cn1LoopbackListeners objectForKey:key]; + if(existing != nil) { + return [existing intValue]; + } + int fd = socket(AF_INET, SOCK_STREAM, 0); + if(fd < 0) { + *errorOut = @"Could not create a socket"; + return -1; + } + int yes = 1; + setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(yes)); + struct sockaddr_in addr; + memset(&addr, 0, sizeof(addr)); + addr.sin_family = AF_INET; + addr.sin_port = htons((uint16_t)port); + // INADDR_LOOPBACK, never INADDR_ANY: this channel must not be reachable from + // the network, only from the device itself. + addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + if(bind(fd, (struct sockaddr*)&addr, sizeof(addr)) < 0) { + *errorOut = [NSString stringWithFormat:@"Could not bind port %i on loopback", port]; + close(fd); + return -1; + } + if(listen(fd, 50) < 0) { + *errorOut = [NSString stringWithFormat:@"Could not listen on port %i", port]; + close(fd); + return -1; + } + [_cn1LoopbackListeners setObject:[NSNumber numberWithInt:fd] forKey:key]; + return fd; + } +} + +-(BOOL)listenLoopback:(int)param{ + NSString* bindError = nil; + int listenFd = [SocketImpl loopbackListenerForPort:param errorOut:&bindError]; + if(listenFd < 0) { + errorMessage = bindError; + errorCode = -1; + connected = NO; + return NO; + } + + // accept() blocks; hand the VM back this thread while it does. + int clientFd; + _yield(); + clientFd = accept(listenFd, NULL, NULL); + _resume(); + if(clientFd < 0) { + errorMessage = @"Accept failed"; + errorCode = -1; + connected = NO; + return NO; + } + + CFReadStreamRef readStream = NULL; + CFWriteStreamRef writeStream = NULL; + CFStreamCreatePairWithSocket(NULL, (CFSocketNativeHandle)clientFd, &readStream, &writeStream); + if(readStream == NULL || writeStream == NULL) { + close(clientFd); + errorMessage = @"Could not wrap the accepted socket"; + errorCode = -1; + connected = NO; + return NO; + } + // Hand ownership of the descriptor to the streams so closing them closes it. + CFReadStreamSetProperty(readStream, kCFStreamPropertyShouldCloseNativeSocket, kCFBooleanTrue); + CFWriteStreamSetProperty(writeStream, kCFStreamPropertyShouldCloseNativeSocket, kCFBooleanTrue); + + inputStream = (BRIDGE_CAST NSInputStream *)readStream; + outputStream = (BRIDGE_CAST NSOutputStream *)writeStream; + [inputStream setDelegate:self]; + [outputStream setDelegate:self]; + [inputStream scheduleInRunLoop:[NSRunLoop currentRunLoop] forMode:NSDefaultRunLoopMode]; + [outputStream scheduleInRunLoop:[NSRunLoop currentRunLoop] forMode:NSDefaultRunLoopMode]; + [inputStream open]; + [outputStream open]; + while ([outputStream streamStatus] == NSStreamStatusOpening) { + _yield(); + usleep(10000); + _resume(); + } + while ([inputStream streamStatus] == NSStreamStatusOpening) { + _yield(); + usleep(10000); + _resume(); + } + connected = ![self isInputShutdown] && ![self isOutputShutdown]; + return connected; +} + -(BOOL)isConnected{ return connected; } diff --git a/Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java b/Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java index d8033ec0b07..456a25382c1 100644 --- a/Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java +++ b/Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java @@ -11843,8 +11843,24 @@ public Object connectSocket(String host, int port, int connectTimeout) { @Override public Object listenSocket(int port) { + // Wildcard-bound server sockets stay unsupported on iOS; see + // listenSocketLoopback, which binds the loopback interface only. return null; } + + @Override + public boolean isLoopbackServerSocketAvailable() { + return true; + } + + @Override + public Object listenSocketLoopback(int port) { + long peer = nativeInstance.listenSocketLoopback(port); + if(peer == 0) { + return null; + } + return new Long(peer); + } @Override public String getHostOrIP() { diff --git a/Ports/iOSPort/src/com/codename1/impl/ios/IOSNative.java b/Ports/iOSPort/src/com/codename1/impl/ios/IOSNative.java index 4e8afbc50ac..81d988b7981 100644 --- a/Ports/iOSPort/src/com/codename1/impl/ios/IOSNative.java +++ b/Ports/iOSPort/src/com/codename1/impl/ios/IOSNative.java @@ -797,6 +797,7 @@ native void gl3dDrawArrays(long contextPeer, long pipelinePeer, long vboPeer, in public native int getVKBWidth(); public native long connectSocket(String host, int port, int connectTimeout); + public native long listenSocketLoopback(int port); public native String getHostOrIP(); public native void disconnectSocket(long socket); public native boolean isSocketConnected(long socket); diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java new file mode 100644 index 00000000000..c05b9b88dea --- /dev/null +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java @@ -0,0 +1,171 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.codename1.mcp; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.PipedInputStream; +import java.io.PipedOutputStream; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/// Framing and session handling for the portable MCP socket transport — the one that +/// lets an agent attach to an app running on a device, not only on the desktop. +/// +/// Drives the transport's own streams directly rather than through +/// {@code Socket.listenLoopback}: what is under test here is the message framing and the +/// attach / detach / close handshake. Going through the socket layer would instead be +/// exercising the platform's thread scheduling. +class MCPLoopbackSocketTransportTest { + + private MCPLoopbackSocketTransport transport; + + @AfterEach + void closeTransport() { + // The transport is process-wide; leaving one open would fail the next test. + if (transport != null) { + transport.close(); + transport = null; + } + } + + private MCPLoopbackSocketTransport attached(String clientBytes) throws IOException { + transport = new MCPLoopbackSocketTransport(47811); + transport.attach(new ByteArrayInputStream(clientBytes.getBytes("UTF-8")), + new ByteArrayOutputStream()); + return transport; + } + + @Test + void readsWholeLineDelimitedMessages() throws Exception { + MCPLoopbackSocketTransport t = attached("{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"initialize\"}\n"); + String message = t.readMessage(); + assertNotNull(message); + assertEquals("{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"initialize\"}", message, + "the newline delimits the frame and is not part of the message"); + } + + @Test + void splitsConsecutiveMessagesArrivingTogether() throws Exception { + // Both messages in one chunk: framing must not depend on packet boundaries. + MCPLoopbackSocketTransport t = attached("{\"id\":1}\n{\"id\":2}\n"); + assertEquals("{\"id\":1}", t.readMessage()); + assertEquals("{\"id\":2}", t.readMessage()); + } + + @Test + void toleratesCarriageReturnsBeforeTheDelimiter() throws Exception { + MCPLoopbackSocketTransport t = attached("{\"id\":1}\r\n"); + assertEquals("{\"id\":1}", t.readMessage(), + "a CRLF-writing client must not leave a stray carriage return in the JSON"); + } + + @Test + void writesMessagesWithATrailingDelimiter() throws Exception { + transport = new MCPLoopbackSocketTransport(47811); + ByteArrayOutputStream toClient = new ByteArrayOutputStream(); + transport.attach(new ByteArrayInputStream(new byte[0]), toClient); + transport.writeMessage("{\"id\":7}"); + assertEquals("{\"id\":7}\n", new String(toClient.toByteArray(), "UTF-8"), + "every reply must be one line so the peer can frame it"); + } + + @Test + void writeFailsWhenNoAgentIsAttached() { + transport = new MCPLoopbackSocketTransport(47811); + assertThrows(IOException.class, () -> transport.writeMessage("{\"id\":1}"), + "writing with nobody attached is an error, not a silent no-op"); + } + + @Test + void aDisconnectedClientDoesNotEndTheServer() throws Exception { + // The first client's stream ends; the transport must wait for the next agent + // instead of reporting end-of-transport, so reconnects keep working. + final MCPLoopbackSocketTransport t = new MCPLoopbackSocketTransport(47811); + transport = t; + t.attach(new ByteArrayInputStream(new byte[0]), new ByteArrayOutputStream()); + + final String[] out = new String[1]; + Thread reader = new Thread(() -> { + try { + out[0] = t.readMessage(); + } catch (IOException ignored) { + // surfaces as a null message below + } + }); + reader.start(); + Thread.sleep(150); + assertTrue(reader.isAlive(), "the reader should be waiting for the next client"); + + t.attach(new ByteArrayInputStream("{\"id\":9}\n".getBytes("UTF-8")), + new ByteArrayOutputStream()); + reader.join(3000); + assertEquals("{\"id\":9}", out[0], "the reconnecting agent's message should arrive"); + } + + @Test + void closeUnblocksAPendingRead() throws Exception { + final MCPLoopbackSocketTransport t = new MCPLoopbackSocketTransport(47811); + transport = t; + final String[] out = new String[1]; + final boolean[] done = new boolean[1]; + Thread reader = new Thread(() -> { + try { + out[0] = t.readMessage(); + } catch (IOException ignored) { + // end of transport is reported as a null message + } + done[0] = true; + }); + reader.start(); + Thread.sleep(150); + t.close(); + reader.join(3000); + + assertTrue(done[0], "close must wake a reader that is waiting for a client"); + assertNull(out[0], "end of transport is signalled by a null message"); + } + + @Test + void aPipedClientRoundTrips() throws Exception { + // Closest thing to a real socket without one: the client end is a live pipe, so + // the read blocks until bytes actually arrive rather than hitting a buffer's end. + PipedOutputStream clientWrites = new PipedOutputStream(); + InputStream serverReads = new PipedInputStream(clientWrites, 4096); + transport = new MCPLoopbackSocketTransport(47811); + transport.attach(serverReads, new ByteArrayOutputStream()); + + clientWrites.write("{\"id\":42}\n".getBytes("UTF-8")); + clientWrites.flush(); + assertEquals("{\"id\":42}", transport.readMessage()); + } +} diff --git a/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java b/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java index d9d71e2e30d..d6bea959b24 100644 --- a/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java +++ b/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java @@ -2788,6 +2788,20 @@ public Object listenSocket(int port) { } } + @Override + public boolean isLoopbackServerSocketAvailable() { + return serverSocketAvailable; + } + + /// There is no real network in these tests, so a loopback accept is the same + /// simulated queue as a wildcard one. The distinction under test is the API contract + /// (callers asking for loopback get a socket, and a port that cannot bind loopback + /// refuses), not the kernel-level bind. + @Override + public Object listenSocketLoopback(int port) { + return listenSocket(port); + } + @Override public void disconnectSocket(Object socket) { if (socket instanceof TestSocket) { From 91949e3fa303912ec2ab80d43729e8a8f7629dc3 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 26 Jul 2026 07:33:39 +0300 Subject: [PATCH 02/24] iOS: serve accepted sockets with raw descriptor I/O MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The loopback listener accepted connections but no data ever crossed them. The accepted descriptor was wrapped in a CFStream pair scheduled on [NSRunLoop currentRunLoop] — which is what the client-side connect path does, and correct there — but a server socket is read on a background thread whose run loop never runs, so the streams opened and then delivered nothing. Accepted sockets now use recv/send on the descriptor directly, with FIONREAD for available input and a blocking read (the caller is a reader thread that expects to wait, and a non-blocking nil return would spin). The client path is untouched. Verified end to end on the iOS simulator: an agent attaches to a running app, reads its accessibility tree, and drives it through several screens. Co-Authored-By: Claude Opus 5 (1M context) --- Ports/iOSPort/nativeSources/SocketImpl.h | 4 ++ Ports/iOSPort/nativeSources/SocketImpl.m | 91 +++++++++++++++--------- 2 files changed, 60 insertions(+), 35 deletions(-) diff --git a/Ports/iOSPort/nativeSources/SocketImpl.h b/Ports/iOSPort/nativeSources/SocketImpl.h index 7e81f7f0a66..d6449b2a145 100644 --- a/Ports/iOSPort/nativeSources/SocketImpl.h +++ b/Ports/iOSPort/nativeSources/SocketImpl.h @@ -7,6 +7,10 @@ NSString* errorMessage; int errorCode; BOOL connected; + /// Accepted server-side sockets do raw descriptor I/O rather than going through + /// NSStream: they are serviced on a background thread whose run loop never runs, and + /// a scheduled CFStream would simply never deliver anything. -1 when unused. + int rawFd; } -(BOOL)connect:(NSString*)host port:(int)port timeout:(int)timeout; diff --git a/Ports/iOSPort/nativeSources/SocketImpl.m b/Ports/iOSPort/nativeSources/SocketImpl.m index 2f99b4fc986..1263ced345f 100644 --- a/Ports/iOSPort/nativeSources/SocketImpl.m +++ b/Ports/iOSPort/nativeSources/SocketImpl.m @@ -1,6 +1,7 @@ #import "SocketImpl.h" #include #include +#include #include #include #include @@ -9,6 +10,14 @@ @implementation SocketImpl +-(id)init { + self = [super init]; + if(self != nil) { + rawFd = -1; // stream-backed unless an accept installs a descriptor + } + return self; +} + static void _yield() { #ifdef NEW_CODENAME_ONE_VM CN1_YIELD_THREAD; @@ -86,6 +95,13 @@ -(BOOL)isOutputShutdown{ } -(int)getAvailableInput{ + if(rawFd >= 0) { + int pending = 0; + if(ioctl(rawFd, FIONREAD, &pending) < 0) { + return 0; + } + return pending; + } //return availableValue; return [inputStream hasBytesAvailable] ? 1 : 0; } @@ -127,7 +143,20 @@ +(NSString*)getIP{ -(NSData*)readFromStream{ uint8_t buffer[8192]; int len; - + + if(rawFd >= 0) { + // Blocking recv: the caller is a reader thread and expects to wait for data, + // and returning nil in a spin would burn the CPU. + _yield(); + ssize_t r = recv(rawFd, buffer, sizeof(buffer), 0); + _resume(); + if(r > 0) { + return [NSData dataWithBytes:buffer length:r]; + } + connected = NO; + return nil; + } + if([inputStream hasBytesAvailable]) { len = [inputStream read:buffer maxLength:sizeof(buffer)]; if (len > 0) { @@ -138,10 +167,30 @@ -(NSData*)readFromStream{ } -(void)writeToStream:(NSData*)param{ + if(rawFd >= 0) { + const uint8_t* bytes = (const uint8_t*)[param bytes]; + size_t remaining = [param length]; + while(remaining > 0) { + ssize_t w = send(rawFd, bytes, remaining, 0); + if(w <= 0) { + connected = NO; + return; + } + bytes += w; + remaining -= w; + } + return; + } [outputStream write:[param bytes] maxLength:[param length]]; } -(void)disconnect{ + if(rawFd >= 0) { + close(rawFd); + rawFd = -1; + connected = NO; + return; + } if(inputStream != nil) { [outputStream close]; [outputStream removeFromRunLoop:[NSRunLoop currentRunLoop] forMode:NSDefaultRunLoopMode]; @@ -226,40 +275,12 @@ -(BOOL)listenLoopback:(int)param{ return NO; } - CFReadStreamRef readStream = NULL; - CFWriteStreamRef writeStream = NULL; - CFStreamCreatePairWithSocket(NULL, (CFSocketNativeHandle)clientFd, &readStream, &writeStream); - if(readStream == NULL || writeStream == NULL) { - close(clientFd); - errorMessage = @"Could not wrap the accepted socket"; - errorCode = -1; - connected = NO; - return NO; - } - // Hand ownership of the descriptor to the streams so closing them closes it. - CFReadStreamSetProperty(readStream, kCFStreamPropertyShouldCloseNativeSocket, kCFBooleanTrue); - CFWriteStreamSetProperty(writeStream, kCFStreamPropertyShouldCloseNativeSocket, kCFBooleanTrue); - - inputStream = (BRIDGE_CAST NSInputStream *)readStream; - outputStream = (BRIDGE_CAST NSOutputStream *)writeStream; - [inputStream setDelegate:self]; - [outputStream setDelegate:self]; - [inputStream scheduleInRunLoop:[NSRunLoop currentRunLoop] forMode:NSDefaultRunLoopMode]; - [outputStream scheduleInRunLoop:[NSRunLoop currentRunLoop] forMode:NSDefaultRunLoopMode]; - [inputStream open]; - [outputStream open]; - while ([outputStream streamStatus] == NSStreamStatusOpening) { - _yield(); - usleep(10000); - _resume(); - } - while ([inputStream streamStatus] == NSStreamStatusOpening) { - _yield(); - usleep(10000); - _resume(); - } - connected = ![self isInputShutdown] && ![self isOutputShutdown]; - return connected; + // Serve this socket with raw descriptor I/O. An accepted connection is read on a + // background thread whose run loop never runs, so a scheduled CFStream would open + // but never deliver a byte. + rawFd = clientFd; + connected = YES; + return YES; } -(BOOL)isConnected{ From 075bb33b542a39b7e870d2b451fd35ad02b93132 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 26 Jul 2026 08:04:09 +0300 Subject: [PATCH 03/24] Address review: port 0 key collision, fail-fast listen, iOS leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four review findings plus the static-analysis and header gates. - The server-socket cache distinguished loopback from wildcard by negating the port. -0 == 0, so on an ephemeral-port listener (port 0) a loopback request could be handed the wildcard-bound socket, defeating the one guarantee this API exists to make. Loopback and wildcard sockets now live in separate maps, so no encoding can collide. Fixed in both the JavaSE and Android ports. - Socket.listenLoopback() documented that it fails when the platform has no loopback support, but it started a listener thread that hit the default implementation's exception and merely logged it — the caller believed it was listening. It now throws IllegalStateException on the calling thread. - The iOS listenSocketLoopback binding released nothing when the bind or accept failed, leaking a SocketImpl per retry. (The adjacent connectSocket has the same pre-existing shape; left alone to keep this change reviewable.) - PMD: the identity comparisons and borrowed-stream locals in the transport are deliberate — a singleton registration slot and streams owned by the connection callback — so they carry NOPMD markers explaining why, rather than relaxing CompareObjectsWithEquals or CloseResource for the whole codebase. - SocketImpl.h/.m never carried a copyright header; modifying them tripped the header gate. Added the standard header. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/io/Socket.java | 10 ++++++++ .../mcp/MCPLoopbackSocketTransport.java | 24 ++++++++++++++----- .../impl/android/AndroidImplementation.java | 14 +++++++---- .../com/codename1/impl/javase/JavaSEPort.java | 15 +++++++----- Ports/iOSPort/nativeSources/IOSNative.m | 3 +++ Ports/iOSPort/nativeSources/SocketImpl.h | 22 +++++++++++++++++ Ports/iOSPort/nativeSources/SocketImpl.m | 22 +++++++++++++++++ 7 files changed, 93 insertions(+), 17 deletions(-) diff --git a/CodenameOne/src/com/codename1/io/Socket.java b/CodenameOne/src/com/codename1/io/Socket.java index 1c7fb6f66c2..082a0471207 100644 --- a/CodenameOne/src/com/codename1/io/Socket.java +++ b/CodenameOne/src/com/codename1/io/Socket.java @@ -206,7 +206,17 @@ public static StopListening listen(final int port, final Class scClass) { /// #### Returns /// /// StopListening instance that allows the caller to stop listening + /// + /// #### Throws + /// + /// - `IllegalStateException`: if this platform cannot bind a loopback server socket. + /// Thrown here rather than on the listener thread so a caller cannot walk away + /// believing it is listening when nothing ever bound. public static StopListening listenLoopback(final int port, final Class scClass) { + if (!isLoopbackServerSocketSupported()) { + throw new IllegalStateException("This platform cannot bind a loopback server " + + "socket; check Socket.isLoopbackServerSocketSupported() first"); + } return listenImpl(port, scClass, true); } diff --git a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java index d54444491e9..cb934227556 100644 --- a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java +++ b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java @@ -85,7 +85,8 @@ public void open() throws IOException { + "MCP agent cannot attach to it"); } synchronized (MCPLoopbackSocketTransport.class) { - if (active != null && active != this) { + // Identity, not equality: is the open transport a DIFFERENT instance? + if (active != null && active != this) { // NOPMD identity is the question throw new IOException("Another MCP socket transport is already open on port " + active.port); } @@ -107,7 +108,9 @@ void attach(InputStream is, OutputStream os) { /// Called when a client's session ends. void detach(InputStream is) { synchronized (lock) { - if (in == is) { + // Detach only if this is still the very same stream we attached, so a newer + // client is not torn down by a stale one finishing late. + if (in == is) { // NOPMD identity: same stream we attached? in = null; out = null; lock.notifyAll(); @@ -118,7 +121,10 @@ void detach(InputStream is) { @Override public String readMessage() throws IOException { while (true) { - InputStream stream; + // Borrowed reference: read the field under the lock, use it outside. The + // stream is owned by the connection callback and closed in close(); closing + // it here would end the session after a single message. + InputStream stream; // NOPMD borrowed, not owned by this method synchronized (lock) { while (in == null && !closed) { try { @@ -176,7 +182,9 @@ private static String toUtf8(ByteArrayOutputStream buffer) throws IOException { @Override public void writeMessage(String message) throws IOException { - OutputStream stream; + // Borrowed reference (see readMessage): this writes one message, it does not own + // the session and must not close the stream. + OutputStream stream; // NOPMD borrowed, not owned by this method synchronized (lock) { stream = out; } @@ -197,7 +205,9 @@ private boolean isClosed() { @Override public void close() { Socket.StopListening l; - InputStream is; + // Taken out of the field under the lock precisely so it can be closed below, + // outside the lock: closing is what unblocks a reader parked in read(). + InputStream is; // NOPMD closed below, deliberately outside the lock synchronized (lock) { closed = true; l = listening; @@ -208,7 +218,9 @@ public void close() { lock.notifyAll(); } synchronized (MCPLoopbackSocketTransport.class) { - if (active == this) { + // Clear the registration only if it is still ours; a transport opened after + // this one must keep its slot. + if (active == this) { // NOPMD identity: is the slot still ours? active = null; } } diff --git a/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java b/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java index e0d87bf75b2..dcd9ed62037 100644 --- a/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java +++ b/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java @@ -11756,6 +11756,7 @@ private synchronized ServerSockets getServerSockets() { class ServerSockets { Map socks = new HashMap(); + Map loopbackSocks = new HashMap(); public synchronized ServerSocket get(int port) throws IOException { return get(port, false); @@ -11764,17 +11765,20 @@ public synchronized ServerSocket get(int port) throws IOException { /** * When loopbackOnly is set the socket binds 127.0.0.1 rather than the wildcard * address, so the channel isn't published on every network interface. The two - * are cached separately: a port that is already bound to the wildcard address - * must never be handed back to a caller that asked for loopback. + * are cached in SEPARATE maps: a port that is already bound to the wildcard + * address must never be handed back to a caller that asked for loopback. + * Distinguishing them by sign within one map would collide on port 0, the + * ephemeral-port request, where -0 == 0. */ public synchronized ServerSocket get(int port, boolean loopbackOnly) throws IOException { - Integer key = Integer.valueOf(loopbackOnly ? -port : port); - ServerSocket sock = socks.get(key); + Map cache = loopbackOnly ? loopbackSocks : socks; + Integer key = Integer.valueOf(port); + ServerSocket sock = cache.get(key); if (sock == null || sock.isClosed()) { sock = loopbackOnly ? new ServerSocket(port, 50, InetAddress.getLoopbackAddress()) : new ServerSocket(port); - socks.put(key, sock); + cache.put(key, sock); } return sock; } diff --git a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java index 5512dc3ac71..be34b1389e5 100644 --- a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java +++ b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java @@ -17122,23 +17122,26 @@ private synchronized ServerSockets getServerSockets() { class ServerSockets { Map socks = new HashMap(); + Map loopbackSocks = new HashMap(); public synchronized ServerSocket get(int port) throws IOException { return get(port, false); } /// When loopbackOnly is set the socket binds 127.0.0.1 instead of the wildcard, so - /// the channel is not published on every network interface. The two are cached - /// separately: a port already bound wildcard must never be handed back to a caller - /// that asked for loopback. + /// the channel is not published on every network interface. The two are cached in + /// SEPARATE maps: a port already bound wildcard must never be handed back to a + /// caller that asked for loopback. Distinguishing them by sign within one map + /// would collide on port 0, the ephemeral-port request, where -0 == 0. public synchronized ServerSocket get(int port, boolean loopbackOnly) throws IOException { - Integer key = Integer.valueOf(loopbackOnly ? -port : port); - ServerSocket sock = socks.get(key); + Map cache = loopbackOnly ? loopbackSocks : socks; + Integer key = Integer.valueOf(port); + ServerSocket sock = cache.get(key); if (sock == null || sock.isClosed()) { sock = loopbackOnly ? new ServerSocket(port, 50, InetAddress.getLoopbackAddress()) : new ServerSocket(port); - socks.put(key, sock); + cache.put(key, sock); } return sock; } diff --git a/Ports/iOSPort/nativeSources/IOSNative.m b/Ports/iOSPort/nativeSources/IOSNative.m index 7ced37af245..db7d4e4aa39 100644 --- a/Ports/iOSPort/nativeSources/IOSNative.m +++ b/Ports/iOSPort/nativeSources/IOSNative.m @@ -10853,6 +10853,9 @@ JAVA_LONG com_codename1_impl_ios_IOSNative_listenSocketLoopback___int(CN1_THREAD if(b) { return (JAVA_LONG)impl; } + // ownership is transferred to Java only on success, so a failed bind or accept must + // release here or every retry leaks the peer + [impl release]; return (JAVA_LONG)JAVA_NULL; } diff --git a/Ports/iOSPort/nativeSources/SocketImpl.h b/Ports/iOSPort/nativeSources/SocketImpl.h index d6449b2a145..a397eeeac27 100644 --- a/Ports/iOSPort/nativeSources/SocketImpl.h +++ b/Ports/iOSPort/nativeSources/SocketImpl.h @@ -1,3 +1,25 @@ +/* + * Copyright (c) 2012, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ #import @interface SocketImpl : NSObject { diff --git a/Ports/iOSPort/nativeSources/SocketImpl.m b/Ports/iOSPort/nativeSources/SocketImpl.m index 1263ced345f..c9c5eb9ea8d 100644 --- a/Ports/iOSPort/nativeSources/SocketImpl.m +++ b/Ports/iOSPort/nativeSources/SocketImpl.m @@ -1,3 +1,25 @@ +/* + * Copyright (c) 2012, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ #import "SocketImpl.h" #include #include From c14cec6bfae027af312b02fd84b1579ecbba8cf7 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 26 Jul 2026 08:36:26 +0300 Subject: [PATCH 04/24] Keep the new sources ASCII-only The Android port compiles CodenameOne/src through ant javac with ASCII encoding, so the em dashes in the doc comments I added were hard errors ("unmappable character for encoding ASCII"), not warnings. Replaced with hyphens; master carried no non-ASCII in these files. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/com/codename1/impl/CodenameOneImplementation.java | 2 +- CodenameOne/src/com/codename1/io/Socket.java | 2 +- CodenameOne/src/com/codename1/mcp/MCP.java | 6 +++--- .../src/com/codename1/mcp/MCPLoopbackSocketTransport.java | 4 ++-- .../com/codename1/mcp/MCPLoopbackSocketTransportTest.java | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java b/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java index e790b48d89b..3a4917dbb22 100644 --- a/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java +++ b/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java @@ -9932,7 +9932,7 @@ public boolean isLoopbackServerSocketAvailable() { } /// Listens on the given port, bound to the loopback interface only, and blocks until - /// a connection arrives — the accept semantics of [#listenSocket(int)], with a + /// a connection arrives - the accept semantics of [#listenSocket(int)], with a /// narrower bind. /// /// #### Parameters diff --git a/CodenameOne/src/com/codename1/io/Socket.java b/CodenameOne/src/com/codename1/io/Socket.java index 082a0471207..42a4765b220 100644 --- a/CodenameOne/src/com/codename1/io/Socket.java +++ b/CodenameOne/src/com/codename1/io/Socket.java @@ -190,7 +190,7 @@ public static StopListening listen(final int port, final Class scClass) { /// [#listen(int, Class)]: `scClass` is instantiated per incoming connection and /// must have a public no-argument constructor. /// - /// Use this for anything that is local by nature — a debug or automation channel, + /// Use this for anything that is local by nature - a debug or automation channel, /// an on-device tool talking to a companion process. Callers that genuinely want to /// serve the network should use [#listen(int, Class)] and say so. /// diff --git a/CodenameOne/src/com/codename1/mcp/MCP.java b/CodenameOne/src/com/codename1/mcp/MCP.java index 7c5db2d46ea..b30b0990ce9 100644 --- a/CodenameOne/src/com/codename1/mcp/MCP.java +++ b/CodenameOne/src/com/codename1/mcp/MCP.java @@ -79,7 +79,7 @@ public static void setSocketTransportFactory(SocketTransportFactory factory) { } /// Whether the PORT supplied a transport of its own. Distinct from - /// [#isSocketSupported()], which also counts the portable fallback — the fallback + /// [#isSocketSupported()], which also counts the portable fallback - the fallback /// must not see itself as already installed. static boolean hasPlatformSocketTransport() { return socketTransportFactory != null; @@ -90,7 +90,7 @@ public static boolean isStdioSupported() { return stdioTransportFactory != null; } - /// Whether a loopback socket transport is available on this platform — either one the + /// Whether a loopback socket transport is available on this platform - either one the /// port registered itself, or the portable one, which needs /// [com.codename1.io.Socket#isLoopbackServerSocketSupported()]. public static boolean isSocketSupported() { @@ -127,7 +127,7 @@ public static synchronized MCPServer startStdioServer() { /// Requires a platform socket transport factory (registered by the JavaSE port). public static synchronized MCPServer startSocketServer(int port) { // Ports that did not register a transport of their own fall back to the portable - // one, which binds loopback through com.codename1.io.Socket — so attaching to a + // one, which binds loopback through com.codename1.io.Socket - so attaching to a // running app on a device or simulator works, not only on the desktop. MCPLoopbackSocketTransport.registerIfPlatformHasNone(); if (socketTransportFactory == null) { diff --git a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java index cb934227556..b412a647dea 100644 --- a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java +++ b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java @@ -31,7 +31,7 @@ import java.io.OutputStream; /// Loopback socket MCP transport built on the portable [com.codename1.io.Socket] API, so -/// an agent can attach to a running app on ANY port that can bind the loopback interface — +/// an agent can attach to a running app on ANY port that can bind the loopback interface - /// a device or simulator, not only the desktop. /// /// The channel is bound to loopback only. That is the security boundary: an attaching @@ -40,7 +40,7 @@ /// fallback; if the port cannot bind loopback this transport refuses to open. /// /// The listening socket stays bound across client sessions, so an agent may disconnect and -/// reconnect — each drive call is typically its own short-lived connection — without the +/// reconnect - each drive call is typically its own short-lived connection - without the /// server thread exiting. public final class MCPLoopbackSocketTransport implements MCPTransport { /// Lets [Connection], which the socket API instantiates reflectively via a public diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java index c05b9b88dea..7d242406e75 100644 --- a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java @@ -38,7 +38,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -/// Framing and session handling for the portable MCP socket transport — the one that +/// Framing and session handling for the portable MCP socket transport - the one that /// lets an agent attach to an app running on a device, not only on the desktop. /// /// Drives the transport's own streams directly rather than through From 3f4b10444ba23b4170ed5115d78d12aa98b466cf Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 26 Jul 2026 08:48:04 +0300 Subject: [PATCH 05/24] Block the MCP socket server on release builds Serving MCP over a socket used to be JavaSE only, which meant simulator and desktop tooling: inherently a development environment. Now that the transport works wherever loopback can be bound, the same call can bind inside an app on a phone, and the risk profile is not the same. An attached agent reads the screen and drives the UI, and loopback is shared by everything on the device rather than being private to one app. So on a shipped build, any OTHER installed app could connect to the port and drive yours. Nothing binds unless the app calls a starter, and an unused MCP is stripped by the VM, so this cannot happen by accident -- but a starter left in the code is exactly how a debugging facility reaches production. startSocketServer now refuses on a release build. A development build is: - Android: the package carries FLAG_DEBUGGABLE, which the build sets for a debug variant and clears for a release variant. - iOS: the provisioning profile grants get-task-allow, the entitlement that lets a debugger attach. Development and ad-hoc profiles carry it, App Store and enterprise profiles do not. Unlike the Xcode DEBUG macro this does not depend on which configuration the build server compiled. - JavaSE: always, since that port is the simulator, the designer and the desktop tooling, and its MCP menu already serves agents from there. The default for a port that cannot tell is "release", so the answer errs towards withholding the facility rather than exposing it. setAllowOnReleaseBuilds(true) lifts the block for a build that ships to a controlled fleet. The underlying question is more broadly useful than MCP, so it is exposed as Display.isDebuggableBuild() and documented for gating any development-only facility. Verified in the built artifacts of all three ports, not just at the source level; the iOS native function was clang syntax checked for device and simulator targets. Guide chapter updated with a "Development builds only" section. Co-Authored-By: Claude Opus 5 (1M context) --- .../impl/CodenameOneImplementation.java | 16 +++ CodenameOne/src/com/codename1/mcp/MCP.java | 56 ++++++++- CodenameOne/src/com/codename1/ui/Display.java | 16 +++ .../impl/android/AndroidImplementation.java | 15 +++ .../com/codename1/impl/javase/JavaSEPort.java | 10 ++ Ports/iOSPort/nativeSources/IOSNative.m | 39 ++++++ .../codename1/impl/ios/IOSImplementation.java | 5 + .../src/com/codename1/impl/ios/IOSNative.java | 1 + .../developerguide/snippets/McpSnippets.java | 17 +++ .../developer-guide/MCP-Headless-API.asciidoc | 25 +++- .../mcp/MCPReleaseBuildGateTest.java | 118 ++++++++++++++++++ .../TestCodenameOneImplementation.java | 12 ++ 12 files changed, 327 insertions(+), 3 deletions(-) create mode 100644 maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java diff --git a/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java b/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java index 3a4917dbb22..9566424bc85 100644 --- a/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java +++ b/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java @@ -9931,6 +9931,22 @@ public boolean isLoopbackServerSocketAvailable() { return false; } + /// Whether this build is a development build rather than a release build headed for + /// an app store: a debuggable Android package, a development provisioned iOS build, + /// or the simulator and desktop tooling. + /// + /// Facilities that are appropriate while developing but not in a shipped app can gate + /// themselves on this. A port that cannot tell should leave the default in place: + /// answering "release" is the safe direction, because it withholds a development + /// facility rather than exposing one in production. + /// + /// #### Returns + /// + /// true if this is a development build + public boolean isDebuggableBuild() { + return false; + } + /// Listens on the given port, bound to the loopback interface only, and blocks until /// a connection arrives - the accept semantics of [#listenSocket(int)], with a /// narrower bind. diff --git a/CodenameOne/src/com/codename1/mcp/MCP.java b/CodenameOne/src/com/codename1/mcp/MCP.java index b30b0990ce9..9397bfcb046 100644 --- a/CodenameOne/src/com/codename1/mcp/MCP.java +++ b/CodenameOne/src/com/codename1/mcp/MCP.java @@ -23,13 +23,16 @@ package com.codename1.mcp; import com.codename1.ai.Tool; +import com.codename1.ui.Display; /// Public entry point for the Codename One MCP headless API. Invoking a starter here is /// the enablement; there is no build hint or property to toggle. A single server /// instance exists per process. /// -/// This API is supported by the Codename One JavaSE port, which powers the simulator and -/// the desktop tooling. It is not part of a packaged application build. +/// The stdio transport is supported by the Codename One JavaSE port, which powers the +/// simulator and the desktop tooling. The socket transport also serves a build running on +/// a device, on any port that can bind the loopback interface. It is blocked on a release +/// build - see [#setAllowOnReleaseBuilds(boolean)]. /// /// #### Typical usage /// @@ -47,6 +50,8 @@ public final class MCP { private static MCPServer server; private static StdioTransportFactory stdioTransportFactory; private static SocketTransportFactory socketTransportFactory; + /// Lifts the release build block; see [#setAllowOnReleaseBuilds(boolean)]. + private static boolean allowOnReleaseBuilds; private MCP() { } @@ -125,7 +130,11 @@ public static synchronized MCPServer startStdioServer() { /// Starts a loopback socket server so an agent can attach to this running process. /// Requires a platform socket transport factory (registered by the JavaSE port). + /// + /// Refuses to bind on a RELEASE build, throwing [IllegalStateException] - see + /// [#setAllowOnReleaseBuilds(boolean)] for why, and for the deliberate override. public static synchronized MCPServer startSocketServer(int port) { + checkAllowedOnThisBuild(); // Ports that did not register a transport of their own fall back to the portable // one, which binds loopback through com.codename1.io.Socket - so attaching to a // running app on a device or simulator works, not only on the desktop. @@ -140,6 +149,49 @@ public static synchronized MCPServer startSocketServer(int port) { return s; } + /// Permits the socket server to bind on a RELEASE build. Off by default, and turning + /// it on should be a deliberate, reviewed decision. + /// + /// The reason for the default: an attached agent can read the screen and drive the + /// user interface, and the loopback interface is shared by everything on the device, + /// not private to one application. On a phone that means any OTHER app installed + /// alongside yours can connect to the port and drive your application. That is a fine + /// trade while developing - it is how an agent attaches to a running app - and a poor + /// one in an app a user installs. + /// + /// A development build is detected as a debuggable Android package, a development + /// provisioned iOS build, or the simulator and desktop tooling. Anything else is + /// treated as a release build, so a port that cannot tell withholds the server rather + /// than exposing it. + /// + /// Set this only for a build that ships to a controlled fleet - a kiosk, a test lab, + /// an enterprise deployment - where the device itself is trusted. + /// + /// #### Parameters + /// + /// - `allow`: true to permit the socket server on a release build + public static synchronized void setAllowOnReleaseBuilds(boolean allow) { + allowOnReleaseBuilds = allow; + } + + /// Whether the socket server may bind on a release build. + public static synchronized boolean isAllowedOnReleaseBuilds() { + return allowOnReleaseBuilds; + } + + /// Throws unless this build may serve MCP over a socket. + private static void checkAllowedOnThisBuild() { + if (allowOnReleaseBuilds || Display.getInstance().isDebuggableBuild()) { + return; + } + throw new IllegalStateException( + "The MCP socket server is blocked on a release build. An attached agent can " + + "read the screen and drive the UI, and any other app on the device " + + "can reach the loopback port. Use a development build, or call " + + "MCP.setAllowOnReleaseBuilds(true) if this build ships to a " + + "controlled fleet."); + } + /// Stops the shared server if it is running. public static synchronized void stop() { if (server != null) { diff --git a/CodenameOne/src/com/codename1/ui/Display.java b/CodenameOne/src/com/codename1/ui/Display.java index 83e61ccb8e3..5fc2ceed4b1 100644 --- a/CodenameOne/src/com/codename1/ui/Display.java +++ b/CodenameOne/src/com/codename1/ui/Display.java @@ -6695,6 +6695,22 @@ public boolean isSimulator() { return impl.isSimulator(); } + /// Whether this build is a development build rather than a release build headed for + /// an app store: a debuggable Android package, a development provisioned iOS build, + /// or the simulator and desktop tooling. + /// + /// This is broader than [#isSimulator()], which is true only in the simulator. Use it + /// to gate a facility that belongs in a build you are working on but not in one a user + /// installs. A port that cannot tell reports a release build, so the answer errs + /// towards withholding the facility. + /// + /// #### Returns + /// + /// true if this is a development build + public boolean isDebuggableBuild() { + return impl.isDebuggableBuild(); + } + /// Creates an audio media that can be played in the background. /// /// #### Parameters diff --git a/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java b/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java index dcd9ed62037..5d4a6a667ab 100644 --- a/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java +++ b/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java @@ -11965,6 +11965,21 @@ public Object listenSocketLoopback(int port) { return new SocketImpl().listen(port, true); } + /** + * A debuggable package is one built for development: the flag is set by the + * build for a debug variant and cleared for a release variant, so this reads the + * distinction straight off the installed application rather than guessing. + */ + @Override + public boolean isDebuggableBuild() { + Context ctx = getContext(); + if (ctx == null) { + return false; + } + ApplicationInfo info = ctx.getApplicationInfo(); + return info != null && (info.flags & ApplicationInfo.FLAG_DEBUGGABLE) != 0; + } + @Override public String getHostOrIP() { try { diff --git a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java index be34b1389e5..80df9157f72 100644 --- a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java +++ b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java @@ -9723,6 +9723,16 @@ public boolean isSimulator() { // differentiate simulator from JavaSE port and detect designer return designMode || getSkin() != null || widthLabel != null; } + + /// The JavaSE port is the development environment: the simulator, the designer and the + /// desktop tooling all run on it. A desktop application packaged for distribution runs + /// on it too, which is why this is the port where the answer is a judgement rather than + /// a fact read off the build - and the judgement matches the historical behaviour of + /// the tooling that already serves agents from here. + @Override + public boolean isDebuggableBuild() { + return true; + } /** * @inheritDoc diff --git a/Ports/iOSPort/nativeSources/IOSNative.m b/Ports/iOSPort/nativeSources/IOSNative.m index db7d4e4aa39..a52806357d3 100644 --- a/Ports/iOSPort/nativeSources/IOSNative.m +++ b/Ports/iOSPort/nativeSources/IOSNative.m @@ -10859,6 +10859,45 @@ JAVA_LONG com_codename1_impl_ios_IOSNative_listenSocketLoopback___int(CN1_THREAD return (JAVA_LONG)JAVA_NULL; } +JAVA_BOOLEAN com_codename1_impl_ios_IOSNative_isDebuggableBuild___R_boolean(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject) { +#if TARGET_OS_SIMULATOR + // Nothing ships from the simulator, so it is always a development build. It also has + // no provisioning profile to inspect, which is what the device path relies on. + return JAVA_TRUE; +#else + POOL_BEGIN(); + JAVA_BOOLEAN result = JAVA_FALSE; + // get-task-allow is the entitlement that lets a debugger attach. Development and + // ad-hoc profiles carry it; App Store and enterprise distribution profiles do not, so + // it is the honest "is this a build I am working on" signal, and unlike the Xcode + // DEBUG macro it does not depend on which configuration the build server compiled. + NSString* path = [[NSBundle mainBundle] pathForResource:@"embedded" ofType:@"mobileprovision"]; + if(path != nil) { + NSData* data = [NSData dataWithContentsOfFile:path]; + if(data != nil) { + // The profile is CMS signed, so it is not parseable as a plist; the embedded + // XML is plain text inside it. Latin-1 never fails on arbitrary bytes. + NSString* text = [[NSString alloc] initWithData:data encoding:NSISOLatin1StringEncoding]; + if(text != nil) { + NSRange key = [text rangeOfString:@"get-task-allow"]; + if(key.location != NSNotFound) { + NSUInteger from = key.location + key.length; + NSUInteger avail = [text length] - from; + NSUInteger span = avail < 64 ? avail : 64; + NSRange after = NSMakeRange(from, span); + if([text rangeOfString:@"" options:0 range:after].location != NSNotFound) { + result = JAVA_TRUE; + } + } + [text release]; + } + } + } + POOL_END(); + return result; +#endif +} + JAVA_OBJECT com_codename1_impl_ios_IOSNative_getHostOrIP__(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject) { POOL_BEGIN(); JAVA_OBJECT o = fromNSString(CN1_THREAD_STATE_PASS_ARG [SocketImpl getIP]); diff --git a/Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java b/Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java index 456a25382c1..c1933a36508 100644 --- a/Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java +++ b/Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java @@ -11853,6 +11853,11 @@ public boolean isLoopbackServerSocketAvailable() { return true; } + @Override + public boolean isDebuggableBuild() { + return nativeInstance.isDebuggableBuild(); + } + @Override public Object listenSocketLoopback(int port) { long peer = nativeInstance.listenSocketLoopback(port); diff --git a/Ports/iOSPort/src/com/codename1/impl/ios/IOSNative.java b/Ports/iOSPort/src/com/codename1/impl/ios/IOSNative.java index 81d988b7981..dbbbb38eb73 100644 --- a/Ports/iOSPort/src/com/codename1/impl/ios/IOSNative.java +++ b/Ports/iOSPort/src/com/codename1/impl/ios/IOSNative.java @@ -798,6 +798,7 @@ native void gl3dDrawArrays(long contextPeer, long pipelinePeer, long vboPeer, in public native long connectSocket(String host, int port, int connectTimeout); public native long listenSocketLoopback(int port); + public native boolean isDebuggableBuild(); public native String getHostOrIP(); public native void disconnectSocket(long socket); public native boolean isSocketConnected(long socket); diff --git a/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/McpSnippets.java b/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/McpSnippets.java index f0249d997e8..3f6795f4e3b 100644 --- a/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/McpSnippets.java +++ b/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/McpSnippets.java @@ -26,6 +26,7 @@ import com.codename1.ai.ToolHandler; import com.codename1.mcp.MCP; import com.codename1.mcp.MCPVerbosity; +import com.codename1.ui.Display; /** Compiled source snippets for the MCP headless API guide chapter. */ public class McpSnippets { @@ -49,6 +50,22 @@ public void startStdioServer() { // end::mcp-start-stdio[] } + public void allowOnReleaseBuilds() { + // tag::mcp-allow-release[] + // Only for a build that ships to devices you control, such as a kiosk fleet: + MCP.setAllowOnReleaseBuilds(true); + MCP.startSocketServer(8765); + // end::mcp-allow-release[] + } + + public void startWhenDevelopmentBuild() { + // tag::mcp-guard-start[] + if (Display.getInstance().isDebuggableBuild()) { + MCP.startSocketServer(8765); + } + // end::mcp-guard-start[] + } + public void publishTool(final String signedInUser) { // tag::mcp-add-tool[] MCP.addTool(new Tool( diff --git a/docs/developer-guide/MCP-Headless-API.asciidoc b/docs/developer-guide/MCP-Headless-API.asciidoc index 07fc5bba9e2..a120ba995ea 100644 --- a/docs/developer-guide/MCP-Headless-API.asciidoc +++ b/docs/developer-guide/MCP-Headless-API.asciidoc @@ -4,7 +4,7 @@ The Model Context Protocol (MCP) headless API lets an application expose itself The API reuses the accessibility semantics tree. The same immutable tree that describes the screen to VoiceOver and TalkBack also describes it to the agent, so any application is drivable without extra code. Application specific data and actions are exposed through the existing `com.codename1.ai.Tool` contract. -The API is supported by the JavaSE port, which powers the simulator and the desktop tooling. It's not part of a packaged application build, and it's inert on mobile and web targets. +The socket transport runs anywhere the port can bind the loopback interface, which includes an application running on a device, so an agent can attach to a build on a phone as well as to the simulator. It's blocked on a release build; see <>. The stdio transport needs process standard input and so is supported by the JavaSE port, which powers the simulator and the desktop tooling. == The MCP menu @@ -34,6 +34,29 @@ include::../demos/common/src/main/java/com/codenameone/developerguide/snippets/M The stdio transport is the standard MCP local transport, exchanging newline delimited JSON-RPC messages. While it runs, application logging is redirected away from standard output so it can't corrupt the protocol stream. The stdio transport lives in the JavaSE port because it needs process standard input, which isn't available on every target. +[[Development builds only]] +== Development builds only + +The socket server refuses to bind on a release build and throws an `IllegalStateException` instead. + +The reason is what an attached agent can do. It reads the screen and drives the user interface, and the loopback interface is shared by everything on the device rather than being private to one application. On a phone that means any other installed application can connect to the port and drive yours. That's a reasonable trade while you're developing, because it's how an agent attaches to a running build, and a poor one in an application a user installs. + +A development build is a debuggable Android package, a development provisioned iOS build, or the simulator and the desktop tooling. Anything else counts as a release build, so a port that can't tell the difference withholds the server rather than exposing it. + +Nothing binds a port unless the application calls a starter, and an application that never calls one has no server at all. The gate matters for the case where a starter is left in the code by accident, which is the way a debugging facility usually reaches production. + +The same distinction is available to application code, so a starter can be left in place and simply not fire in a shipped build: + +[source,java] +include::../demos/common/src/main/java/com/codenameone/developerguide/snippets/McpSnippets.java[tag=mcp-guard-start,indent=0] + +A build that ships to devices you control, such as a kiosk fleet, a test lab, or a managed enterprise deployment, can lift the block deliberately: + +[source,java] +include::../demos/common/src/main/java/com/codenameone/developerguide/snippets/McpSnippets.java[tag=mcp-allow-release,indent=0] + +Treat that call the way you'd treat any other decision to expose a control surface, because the device itself becomes the boundary. + == Built-in user interface tools Every server registers a small set of tools that read and drive the screen through the accessibility tree. An agent calls `ui_snapshot` to get the semantics tree of the current form as JSON, including the identifier, role, label, value, and state of each node, and the identifiers of the actions each node supports. diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java new file mode 100644 index 00000000000..52bf86269d8 --- /dev/null +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java @@ -0,0 +1,118 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.codename1.mcp; + +import com.codename1.junit.UITestBase; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/// The MCP socket server must not bind on a release build. An attached agent reads the +/// screen and drives the UI, and loopback is shared by everything on the device, so on a +/// shipped app any other installed app could drive it. +/// +/// These tests never bind a real socket: loopback support is turned OFF, so a start that +/// gets past the gate fails to bind on the server's own reader thread, which only logs. +/// The gate is therefore observable as the difference between throwing on the calling +/// thread and returning a running server. +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +class MCPReleaseBuildGateTest extends UITestBase { + + private static final String GATE = "release build"; + + @AfterEach + void clearOverride() { + MCP.setAllowOnReleaseBuilds(false); + MCP.stop(); + } + + /// Asserts the gate refused, and returns the message so a test can check it names the + /// reason rather than failing with something generic. + private String refusal(boolean debuggable) { + implementation.setDebuggableBuild(debuggable); + implementation.setServerSocketAvailable(false); + return assertThrows(IllegalStateException.class, + () -> MCP.startSocketServer(47899)).getMessage(); + } + + /// Asserts the gate let the start through. Binding still fails (there is no loopback + /// support here) but that happens on the reader thread, so the call returns. + private void assertPassesGate(boolean debuggable) { + implementation.setDebuggableBuild(debuggable); + implementation.setServerSocketAvailable(false); + assertNotNull(MCP.startSocketServer(47899), "start must return the shared server"); + assertTrue(MCP.isRunning(), "the server must be running once the gate is passed"); + } + + @Test + void blocksTheSocketServerOnAReleaseBuild() { + String message = refusal(false); + assertTrue(message.contains(GATE), + "a release build must be refused by the gate, got: " + message); + } + + @Test + void allowsTheSocketServerOnADevelopmentBuild() { + assertPassesGate(true); + } + + @Test + void explicitOverrideLiftsTheBlock() { + MCP.setAllowOnReleaseBuilds(true); + assertPassesGate(false); + } + + @Test + void theOverrideIsOffByDefault() { + // The safe direction has to be the default: a developer who never thinks about + // this ships an app that cannot be driven. + assertFalse(MCP.isAllowedOnReleaseBuilds()); + } + + @Test + void aBlockedStartLeavesNoServerRunning() { + // The gate runs before the transport is registered or opened, so a refusal must + // not leave a half-started server behind. + refusal(false); + assertFalse(MCP.isRunning(), "a refused start must not leave a server running"); + } + + @Test + void theGateIsCheckedBeforeTransportAvailability() { + // Both failure modes apply at once here. The gate must win, otherwise a release + // build on a port that CAN bind would be reported as a platform limitation and + // then quietly succeed once the platform gained support. + implementation.setDebuggableBuild(false); + implementation.setServerSocketAvailable(true); + String message = assertThrows(IllegalStateException.class, + () -> MCP.startSocketServer(47898)).getMessage(); + assertTrue(message.contains(GATE), + "the build gate must be evaluated first, got: " + message); + assertFalse(MCP.isRunning()); + } +} diff --git a/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java b/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java index d6bea959b24..5f20df09358 100644 --- a/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java +++ b/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java @@ -2793,6 +2793,18 @@ public boolean isLoopbackServerSocketAvailable() { return serverSocketAvailable; } + private boolean debuggableBuild = true; + + /// Lets a test choose which side of the release build gate it is on. + public void setDebuggableBuild(boolean debuggableBuild) { + this.debuggableBuild = debuggableBuild; + } + + @Override + public boolean isDebuggableBuild() { + return debuggableBuild; + } + /// There is no real network in these tests, so a loopback accept is the same /// simulated queue as a wildcard one. The distinction under test is the API contract /// (callers asking for loopback get a socket, and a port that cannot bind loopback From af9f4607f7412ebde9898103cc9d3415b0fd246d Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 26 Jul 2026 11:03:37 +0300 Subject: [PATCH 06/24] Address review round 2: fd and signal handling, honest failures Seven review findings, all real. Register the portable transport only when loopback can actually be bound. registerIfPlatformHasNone installed it regardless, so socketTransportFactory became non-null on a platform that cannot bind, startSocketServer reported success, and the open failed later on the server's own thread where no caller could see it. The synchronous "no socket transport" refusal the message promises now actually happens. This is the same defect my own gate test tripped over -- it had to assert the wrong thing to pass. Discard a connection accepted after stop(). The listener re-checks the flag after accept returns, because stop() can be called while the thread is parked inside accept, and the connection that unblocks it belongs to a listener the caller has already abandoned. Previously that last connection was served. Close the accepted descriptor when the peer goes away. recv returning 0 or an error, and a failed send, marked the socket disconnected but left the fd open; the Java side only calls disconnectSocket while the socket still reports itself connected, so the descriptor was stranded for the life of the process. Both paths now go through one closeRawDescriptor. Suppress SIGPIPE on accepted sockets. This process installs a SIGPIPE handler (CodenameOne_GLAppDelegate), so a write to a departed peer -- the ordinary way a client disconnects mid-response -- would surface as a signal rather than the EPIPE the write path is written to handle. SO_NOSIGPIPE on the accepted fd makes it a plain error. Yield across send. accept and recv already hand the thread back to the VM; send blocks once the send buffer fills, and holding the thread there stalled cooperative scheduling for as long as the peer was slow to drain. Also drop an adverb the developer-guide Vale gate rejected. The iOS changes were clang syntax checked against the device and simulator SDKs and confirmed present in the packaged nativeios bundle, not just in the working tree. 32 MCP tests pass, including a new one covering the synchronous refusal. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/io/Socket.java | 9 +++++ .../mcp/MCPLoopbackSocketTransport.java | 7 ++++ Ports/iOSPort/nativeSources/SocketImpl.m | 32 ++++++++++++++-- .../developer-guide/MCP-Headless-API.asciidoc | 2 +- .../mcp/MCPReleaseBuildGateTest.java | 38 +++++++++++++------ 5 files changed, 71 insertions(+), 17 deletions(-) diff --git a/CodenameOne/src/com/codename1/io/Socket.java b/CodenameOne/src/com/codename1/io/Socket.java index 42a4765b220..8afd51d55cc 100644 --- a/CodenameOne/src/com/codename1/io/Socket.java +++ b/CodenameOne/src/com/codename1/io/Socket.java @@ -231,6 +231,15 @@ public void run() { final Object connection = loopbackOnly ? Util.getImplementation().listenSocketLoopback(port) : Util.getImplementation().listenSocket(port); + if (stopped) { + // stop() was called while this thread sat inside accept. The + // connection that unblocked it belongs to a listener the caller + // has already abandoned, so hand it back rather than serving it. + if (connection != null) { + Util.getImplementation().disconnectSocket(connection); + } + break; + } final SocketConnection sc = (SocketConnection) scClass.newInstance(); if (connection != null) { sc.setConnected(true); diff --git a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java index b412a647dea..2155634ccbd 100644 --- a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java +++ b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java @@ -70,6 +70,13 @@ static void registerIfPlatformHasNone() { if (MCP.hasPlatformSocketTransport()) { return; } + if (!Socket.isLoopbackServerSocketSupported()) { + // Registering regardless would be worse than not registering: the caller would + // see a transport, believe the server started, and only find out on the + // server's own thread that nothing can bind. Leaving the factory unset lets + // startSocketServer fail on the calling thread, where it can be handled. + return; + } MCP.setSocketTransportFactory(new MCP.SocketTransportFactory() { @Override public MCPTransport createSocketTransport(int port) { diff --git a/Ports/iOSPort/nativeSources/SocketImpl.m b/Ports/iOSPort/nativeSources/SocketImpl.m index c9c5eb9ea8d..b987dc9906a 100644 --- a/Ports/iOSPort/nativeSources/SocketImpl.m +++ b/Ports/iOSPort/nativeSources/SocketImpl.m @@ -175,7 +175,11 @@ -(NSData*)readFromStream{ if(r > 0) { return [NSData dataWithBytes:buffer length:r]; } - connected = NO; + // End of stream or a read error: close here rather than only marking the socket + // disconnected. The Java side closes a stream by calling disconnectSocket only + // while the socket still reports itself connected, so leaving the descriptor open + // once connected is NO would strand it for the life of the process. + [self closeRawDescriptor]; return nil; } @@ -193,9 +197,14 @@ -(void)writeToStream:(NSData*)param{ const uint8_t* bytes = (const uint8_t*)[param bytes]; size_t remaining = [param length]; while(remaining > 0) { + // send() blocks once the send buffer fills, so hand the thread back to the VM + // across it exactly as the accept and recv paths do; holding it would stall + // cooperative scheduling for as long as the peer is slow to drain. + _yield(); ssize_t w = send(rawFd, bytes, remaining, 0); + _resume(); if(w <= 0) { - connected = NO; + [self closeRawDescriptor]; return; } bytes += w; @@ -206,11 +215,19 @@ -(void)writeToStream:(NSData*)param{ [outputStream write:[param bytes] maxLength:[param length]]; } --(void)disconnect{ +/// Closes the accepted descriptor once, and records the socket as disconnected. Callers +/// reach this from either direction of the connection failing. +-(void)closeRawDescriptor{ if(rawFd >= 0) { close(rawFd); rawFd = -1; - connected = NO; + } + connected = NO; +} + +-(void)disconnect{ + if(rawFd >= 0) { + [self closeRawDescriptor]; return; } if(inputStream != nil) { @@ -300,6 +317,13 @@ -(BOOL)listenLoopback:(int)param{ // Serve this socket with raw descriptor I/O. An accepted connection is read on a // background thread whose run loop never runs, so a scheduled CFStream would open // but never deliver a byte. + // + // Writing to a peer that has gone away raises SIGPIPE by default, and this process + // installs a SIGPIPE handler (CodenameOne_GLAppDelegate), so the ordinary case of a + // client disconnecting mid-write would surface as a signal rather than as the EPIPE + // the write path is written to handle. SO_NOSIGPIPE turns it into a plain error. + int noSigPipe = 1; + setsockopt(clientFd, SOL_SOCKET, SO_NOSIGPIPE, &noSigPipe, sizeof(noSigPipe)); rawFd = clientFd; connected = YES; return YES; diff --git a/docs/developer-guide/MCP-Headless-API.asciidoc b/docs/developer-guide/MCP-Headless-API.asciidoc index a120ba995ea..8dae83896b6 100644 --- a/docs/developer-guide/MCP-Headless-API.asciidoc +++ b/docs/developer-guide/MCP-Headless-API.asciidoc @@ -50,7 +50,7 @@ The same distinction is available to application code, so a starter can be left [source,java] include::../demos/common/src/main/java/com/codenameone/developerguide/snippets/McpSnippets.java[tag=mcp-guard-start,indent=0] -A build that ships to devices you control, such as a kiosk fleet, a test lab, or a managed enterprise deployment, can lift the block deliberately: +A build that ships to devices you control, such as a kiosk fleet, a test lab, or a managed enterprise deployment, can lift the block: [source,java] include::../demos/common/src/main/java/com/codenameone/developerguide/snippets/McpSnippets.java[tag=mcp-allow-release,indent=0] diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java index 52bf86269d8..25eb613d42b 100644 --- a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java @@ -36,10 +36,10 @@ /// screen and drives the UI, and loopback is shared by everything on the device, so on a /// shipped app any other installed app could drive it. /// -/// These tests never bind a real socket: loopback support is turned OFF, so a start that -/// gets past the gate fails to bind on the server's own reader thread, which only logs. -/// The gate is therefore observable as the difference between throwing on the calling -/// thread and returning a running server. +/// The test implementation serves accepts from an in-memory queue rather than a real +/// socket, so a start that gets past the gate reaches a running server without any port +/// being bound. The gate is observable as the difference between throwing on the calling +/// thread and returning that running server. @TestInstance(TestInstance.Lifecycle.PER_CLASS) class MCPReleaseBuildGateTest extends UITestBase { @@ -52,19 +52,19 @@ void clearOverride() { } /// Asserts the gate refused, and returns the message so a test can check it names the - /// reason rather than failing with something generic. + /// reason rather than failing with something generic. Loopback support is ON, so the + /// only reason to refuse is the gate. private String refusal(boolean debuggable) { implementation.setDebuggableBuild(debuggable); - implementation.setServerSocketAvailable(false); + implementation.setServerSocketAvailable(true); return assertThrows(IllegalStateException.class, () -> MCP.startSocketServer(47899)).getMessage(); } - /// Asserts the gate let the start through. Binding still fails (there is no loopback - /// support here) but that happens on the reader thread, so the call returns. + /// Asserts the gate let the start through, leaving a running server behind. private void assertPassesGate(boolean debuggable) { implementation.setDebuggableBuild(debuggable); - implementation.setServerSocketAvailable(false); + implementation.setServerSocketAvailable(true); assertNotNull(MCP.startSocketServer(47899), "start must return the shared server"); assertTrue(MCP.isRunning(), "the server must be running once the gate is passed"); } @@ -105,14 +105,28 @@ void aBlockedStartLeavesNoServerRunning() { @Test void theGateIsCheckedBeforeTransportAvailability() { // Both failure modes apply at once here. The gate must win, otherwise a release - // build on a port that CAN bind would be reported as a platform limitation and - // then quietly succeed once the platform gained support. + // build would be reported as a mere platform limitation and would start serving + // the moment the platform gained loopback support. implementation.setDebuggableBuild(false); - implementation.setServerSocketAvailable(true); + implementation.setServerSocketAvailable(false); String message = assertThrows(IllegalStateException.class, () -> MCP.startSocketServer(47898)).getMessage(); assertTrue(message.contains(GATE), "the build gate must be evaluated first, got: " + message); assertFalse(MCP.isRunning()); } + + @Test + void aPlatformThatCannotBindLoopbackFailsOnTheCallingThread() { + // The portable transport must not register itself on a platform that cannot bind, + // because then the start would look like it succeeded and only fail later on the + // server's own thread, where a caller cannot see it. + implementation.setDebuggableBuild(true); + implementation.setServerSocketAvailable(false); + String message = assertThrows(IllegalStateException.class, + () -> MCP.startSocketServer(47897)).getMessage(); + assertTrue(message.contains("No socket MCP transport"), + "an unbindable platform must say so synchronously, got: " + message); + assertFalse(MCP.isRunning(), "nothing must be left running after that refusal"); + } } From 6b5c6f38b32e3997e327709b9356e2fe6137b338 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 26 Jul 2026 12:07:13 +0300 Subject: [PATCH 07/24] Treat a frame cut short by a disconnect as a disconnect End of stream with no newline returned the partial buffer as if it were a message. For a newline-delimited protocol that is a truncated frame, which means the client went away mid-message -- so the server would parse a broken request, fail to answer it (the peer is already gone), and end its loop. That defeats the one thing this transport is built to do: stay bound so the next agent session can attach. readLine now reports end of stream regardless of how many bytes arrived first, which is the same path a clean disconnect already took. The new test was control-tested against the previous form to confirm it fails there rather than passing for unrelated reasons. Co-Authored-By: Claude Opus 5 (1M context) --- .../mcp/MCPLoopbackSocketTransport.java | 10 ++++-- .../mcp/MCPLoopbackSocketTransportTest.java | 32 +++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java index 2155634ccbd..07d2a81ed86 100644 --- a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java +++ b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java @@ -157,7 +157,8 @@ public String readMessage() throws IOException { } } - /// Reads one newline-delimited UTF-8 message. Returns null at end of stream. + /// Reads one newline-delimited UTF-8 message. Returns null at end of stream, which + /// includes a frame cut short by the client going away. /// /// Read byte at a time rather than through a buffered reader: a buffer would swallow /// bytes belonging to the next message, and this transport hands the same stream back @@ -172,7 +173,12 @@ private String readLine(InputStream stream) throws IOException { return null; // a read error on the client is a disconnect } if (b < 0) { - return buffer.size() == 0 ? null : toUtf8(buffer); + // End of stream with no delimiter: the frame is truncated, so this is a + // disconnect and not a message, however many bytes arrived first. Handing + // the partial frame up would have the server fail to parse it, then fail + // to answer it (the peer is already gone), and end the loop - when the + // whole point of this transport is to stay bound and await a reconnect. + return null; } if (b == '\n') { return toUtf8(buffer); diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java index 7d242406e75..8391048f8af 100644 --- a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java @@ -132,6 +132,38 @@ void aDisconnectedClientDoesNotEndTheServer() throws Exception { assertEquals("{\"id\":9}", out[0], "the reconnecting agent's message should arrive"); } + @Test + void aFrameCutShortByADisconnectIsNotDeliveredAsAMessage() throws Exception { + // A client that dies mid-frame leaves bytes with no delimiter. Delivering those + // would have the server parse a truncated request, fail to answer it (the peer is + // gone) and end the loop; the transport must read it as a plain disconnect and + // stay available, exactly as it does for a clean end of stream. + final MCPLoopbackSocketTransport t = new MCPLoopbackSocketTransport(47811); + transport = t; + t.attach(new ByteArrayInputStream("{\"id\":1,\"method\":\"init".getBytes("UTF-8")), + new ByteArrayOutputStream()); + + final String[] out = new String[1]; + Thread reader = new Thread(() -> { + try { + out[0] = t.readMessage(); + } catch (IOException ignored) { + // surfaces as a null message below + } + }); + reader.start(); + Thread.sleep(150); + assertTrue(reader.isAlive(), + "a truncated frame must leave the reader waiting for the next client, " + + "not hand up a partial message"); + + t.attach(new ByteArrayInputStream("{\"id\":9}\n".getBytes("UTF-8")), + new ByteArrayOutputStream()); + reader.join(3000); + assertEquals("{\"id\":9}", out[0], + "the next agent's whole message should be what finally arrives"); + } + @Test void closeUnblocksAPendingRead() throws Exception { final MCPLoopbackSocketTransport t = new MCPLoopbackSocketTransport(47811); From 9837873286ad5a632bd340518107903489d3d587 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 26 Jul 2026 13:20:30 +0300 Subject: [PATCH 08/24] Only treat a carriage return as delimiter punctuation before a newline readLine dropped every CR it saw, not just the one in a CRLF line ending, so a payload containing a carriage return was silently edited on the way through. A transport should hand up the bytes it was given. The CR is now held back one byte and emitted unless a newline follows it, which keeps CRLF tolerance while leaving content alone. Two tests cover it -- a CR inside the payload, and "\r\r\n" where exactly one of the two carriage returns is content -- and both were control-tested against the previous form to confirm they fail there. The existing CRLF-tolerance test still passes unchanged, which is the property that had to survive this. Also Long.valueOf rather than new Long for the accepted socket peer. Note the allocation saving does not really apply: the cache only covers -128..127 and a native peer never lands there, so this is the better idiom rather than a fix. The adjacent connectSocket has the same shape and is left as pre-existing. Co-Authored-By: Claude Opus 5 (1M context) --- .../mcp/MCPLoopbackSocketTransport.java | 19 ++++++++++++++++--- .../codename1/impl/ios/IOSImplementation.java | 2 +- .../mcp/MCPLoopbackSocketTransportTest.java | 19 +++++++++++++++++++ 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java index 07d2a81ed86..f1b3cf5e981 100644 --- a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java +++ b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java @@ -165,6 +165,10 @@ public String readMessage() throws IOException { /// after a reconnect. private String readLine(InputStream stream) throws IOException { ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + // A carriage return is only part of the delimiter when a newline follows it, so it + // is held back one byte rather than dropped. Dropping every CR would silently edit + // a payload that happened to contain one. + boolean pendingCarriageReturn = false; while (true) { int b; try { @@ -180,12 +184,21 @@ private String readLine(InputStream stream) throws IOException { // whole point of this transport is to stay bound and await a reconnect. return null; } + if (pendingCarriageReturn) { + pendingCarriageReturn = false; + if (b == '\n') { + return toUtf8(buffer); // the CR belonged to a CRLF delimiter + } + buffer.write('\r'); // it belonged to the payload after all + } + if (b == '\r') { + pendingCarriageReturn = true; + continue; + } if (b == '\n') { return toUtf8(buffer); } - if (b != '\r') { - buffer.write(b); - } + buffer.write(b); } } diff --git a/Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java b/Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java index c1933a36508..cf11fbd5763 100644 --- a/Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java +++ b/Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java @@ -11864,7 +11864,7 @@ public Object listenSocketLoopback(int port) { if(peer == 0) { return null; } - return new Long(peer); + return Long.valueOf(peer); } @Override diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java index 8391048f8af..257677f9fdb 100644 --- a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java @@ -89,6 +89,25 @@ void toleratesCarriageReturnsBeforeTheDelimiter() throws Exception { "a CRLF-writing client must not leave a stray carriage return in the JSON"); } + @Test + void keepsACarriageReturnThatIsNotPartOfTheDelimiter() throws Exception { + // Only CR immediately before LF is delimiter punctuation. A CR anywhere else is + // payload, and a transport must hand up the bytes it was given rather than edit + // them, however unlikely the sender. + MCPLoopbackSocketTransport t = attached("{\"a\":\"x\ry\"}\n"); + assertEquals("{\"a\":\"x\ry\"}", t.readMessage(), + "a carriage return inside the payload must survive the framing"); + } + + @Test + void treatsOnlyTheFinalCarriageReturnAsDelimiterPunctuation() throws Exception { + // "\r\r\n" is one payload CR followed by a CRLF delimiter; consuming both CRs + // would corrupt the message, and consuming neither would leave a stray byte. + MCPLoopbackSocketTransport t = attached("ab\r\r\n"); + assertEquals("ab\r", t.readMessage(), + "the trailing CRLF is the delimiter; the CR before it is content"); + } + @Test void writesMessagesWithATrailingDelimiter() throws Exception { transport = new MCPLoopbackSocketTransport(47811); From 8a187a2d7aedafdda44a462368b5ba0968b63ecd Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 26 Jul 2026 15:20:06 +0300 Subject: [PATCH 09/24] Do not strand the MCP registration when the listen fails open() claimed the process-wide `active` slot and only then started listening, so a failure in between left the slot pointing at a transport that never opened. Every later open() would then refuse on the grounds that an agent was already being served. The escape was worse than the stranding. MCPServer's reader thread handles only IOException around open(), and Socket.listenLoopback throws IllegalStateException when loopback support is missing -- so that exception would kill the reader thread before it could clear its running flag, leaving MCP.isRunning() true forever with nothing behind it. The listen is now wrapped: the slot is released and the failure is rethrown as the IOException the server already knows how to handle. close() now shares the same clearActiveIfOurs helper instead of repeating it. Tested by having the platform report loopback support to the first query only, which is the check-then-use race this path guards against. Control-tested against the unguarded form, where it fails with "expected IOException but was IllegalStateException" -- the escape itself. The cause is attached with initCause rather than a two-argument constructor, matching how core already builds IOExceptions (and satisfying PMD's PreserveStackTrace, which the first version of this tripped). Also reset the two test-double fields I added earlier, debuggableBuild included, so a test cannot leak a release-build or one-shot-capability mode into the next. Co-Authored-By: Claude Opus 5 (1M context) --- .../mcp/MCPLoopbackSocketTransport.java | 33 ++++++-- .../mcp/MCPLoopbackTransportOpenTest.java | 79 +++++++++++++++++++ .../TestCodenameOneImplementation.java | 19 +++++ 3 files changed, 123 insertions(+), 8 deletions(-) create mode 100644 maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackTransportOpenTest.java diff --git a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java index f1b3cf5e981..94e7779261c 100644 --- a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java +++ b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java @@ -99,7 +99,29 @@ public void open() throws IOException { } active = this; } - listening = Socket.listenLoopback(port, Connection.class); + try { + listening = Socket.listenLoopback(port, Connection.class); + } catch (RuntimeException ex) { + // Two things go wrong if this escapes. The process-wide registration would stay + // pointing at a transport that never started listening, so every later open() + // would refuse, believing an agent is already served. And the server's reader + // thread only handles IOException around open(), so a runtime exception would + // kill that thread before it could clear its running flag, leaving the server + // permanently "running" with nothing behind it. + clearActiveIfOurs(); + IOException failure = new IOException("Failed to listen on loopback port " + port); + failure.initCause(ex); + throw failure; + } + } + + /// Releases the process-wide registration, but only when it is still this transport's. + private void clearActiveIfOurs() { + synchronized (MCPLoopbackSocketTransport.class) { + if (active == this) { // NOPMD identity: is the slot still ours? + active = null; + } + } } /// Called from the connection callback thread when a client attaches. The previous @@ -243,13 +265,8 @@ public void close() { out = null; lock.notifyAll(); } - synchronized (MCPLoopbackSocketTransport.class) { - // Clear the registration only if it is still ours; a transport opened after - // this one must keep its slot. - if (active == this) { // NOPMD identity: is the slot still ours? - active = null; - } - } + // Only if it is still ours: a transport opened after this one keeps its slot. + clearActiveIfOurs(); if (l != null) { l.stop(); } diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackTransportOpenTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackTransportOpenTest.java new file mode 100644 index 00000000000..3e91993c537 --- /dev/null +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackTransportOpenTest.java @@ -0,0 +1,79 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.codename1.mcp; + +import com.codename1.junit.UITestBase; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; + +import java.io.IOException; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/// What opening the transport does to the process-wide registration, which is the state a +/// failure is most likely to corrupt. Needs a platform implementation, unlike the framing +/// tests, which drive the transport's streams directly. +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +class MCPLoopbackTransportOpenTest extends UITestBase { + + private MCPLoopbackSocketTransport first; + private MCPLoopbackSocketTransport second; + + @AfterEach + void closeTransports() { + // The registration is process-wide, so anything left open fails the next test. + if (first != null) { + first.close(); + first = null; + } + if (second != null) { + second.close(); + second = null; + } + } + + @Test + void aFailedListenNeitherStrandsTheRegistrationNorEscapesAsRuntime() throws Exception { + // The transport checks loopback support and then uses it. Report support to the + // first query only, and the listen that follows the check fails - the real race + // this path guards against. + implementation.setLoopbackSupportedOnlyOnFirstQuery(true); + first = new MCPLoopbackSocketTransport(47881); + + // IOException specifically: the server's reader thread handles only that around + // open(). A RuntimeException would kill the thread before it cleared its running + // flag, leaving the server permanently "running" with nothing behind it. + IOException failure = assertThrows(IOException.class, () -> first.open()); + assertTrue(failure.getMessage().contains("Failed to listen"), + "the failure should name what went wrong, got: " + failure.getMessage()); + + // And the registration must be free, or every later transport would be refused on + // the grounds that an agent is already being served. + implementation.setLoopbackSupportedOnlyOnFirstQuery(false); + implementation.setServerSocketAvailable(true); + second = new MCPLoopbackSocketTransport(47882); + second.open(); + } +} diff --git a/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java b/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java index 5f20df09358..34db0c1c6af 100644 --- a/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java +++ b/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java @@ -1281,6 +1281,9 @@ public void reset() { nativePickerTypeSupported = null; socketAvailable = true; serverSocketAvailable = false; + loopbackSupportedOnlyOnFirstQuery = false; + loopbackQueried = false; + debuggableBuild = true; appHomePath = "file://app/"; hostOrIp = null; resetGalleryTracking(); @@ -2788,8 +2791,24 @@ public Object listenSocket(int port) { } } + private boolean loopbackSupportedOnlyOnFirstQuery; + private boolean loopbackQueried; + + /// Reports loopback support to the first caller and withholds it from every caller + /// after that, which models the capability changing between a check and the use that + /// follows it. Lets a test drive the failure path of a caller that checks first. + public void setLoopbackSupportedOnlyOnFirstQuery(boolean onlyFirst) { + this.loopbackSupportedOnlyOnFirstQuery = onlyFirst; + this.loopbackQueried = false; + } + @Override public boolean isLoopbackServerSocketAvailable() { + if (loopbackSupportedOnlyOnFirstQuery) { + boolean first = !loopbackQueried; + loopbackQueried = true; + return first; + } return serverSocketAvailable; } From 1e01584bebb625de39289de1cca91f4111d6379d Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 26 Jul 2026 16:28:40 +0300 Subject: [PATCH 10/24] Read get-task-allow by parsing the profile, not by proximity The scan looked for "get-task-allow" and then for anywhere in the next 64 characters. In a distribution profile the value that follows is , and the next entitlement is typically beta-reports-active, whose sits about 53 characters later -- inside the window. So an App Store or TestFlight build was classified as a development build, and the release-build gate it feeds would have let the MCP socket server bind in a shipped app. That is the one direction this check must never fail in, and no width of proximity window fixes it; the heuristic was wrong in kind. The CMS wrapper is now cut back to its plist payload, parsed with NSPropertyListSerialization, and the Entitlements dictionary is read for the get-task-allow key. Every path that cannot produce an answer -- no profile, no parseable payload, no such key, unexpected type -- leaves the result false, so an unreadable profile withholds the facility rather than exposing it. Verified by extracting the shipped function into a harness and running it against hand-written profile payloads in Apple's key order: a development profile reads debuggable, a distribution profile with the beta-reports-active following reads release (the old code read it as debuggable), and both a corrupt profile and a missing file read release. Also clang syntax checked against the device and simulator SDKs, and confirmed present in the packaged nativeios bundle. Co-Authored-By: Claude Opus 5 (1M context) --- Ports/iOSPort/nativeSources/IOSNative.m | 47 ++++++++++++++++++------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/Ports/iOSPort/nativeSources/IOSNative.m b/Ports/iOSPort/nativeSources/IOSNative.m index a52806357d3..12e9f1c3b57 100644 --- a/Ports/iOSPort/nativeSources/IOSNative.m +++ b/Ports/iOSPort/nativeSources/IOSNative.m @@ -10875,25 +10875,46 @@ JAVA_BOOLEAN com_codename1_impl_ios_IOSNative_isDebuggableBuild___R_boolean(CN1_ if(path != nil) { NSData* data = [NSData dataWithContentsOfFile:path]; if(data != nil) { - // The profile is CMS signed, so it is not parseable as a plist; the embedded - // XML is plain text inside it. Latin-1 never fails on arbitrary bytes. - NSString* text = [[NSString alloc] initWithData:data encoding:NSISOLatin1StringEncoding]; - if(text != nil) { - NSRange key = [text rangeOfString:@"get-task-allow"]; - if(key.location != NSNotFound) { - NSUInteger from = key.location + key.length; - NSUInteger avail = [text length] - from; - NSUInteger span = avail < 64 ? avail : 64; - NSRange after = NSMakeRange(from, span); - if([text rangeOfString:@"" options:0 range:after].location != NSNotFound) { - result = JAVA_TRUE; + // The profile is CMS signed, so the file as a whole is not a plist, but the + // payload it wraps is one. Cut it out and parse it properly. + // + // Do NOT do this by looking for "get-task-allow" and then hunting nearby for + // : the value that follows it is in a distribution profile, and + // the very next entitlement is often beta-reports-active, whose sits + // about 53 characters later. Any proximity window wide enough to be useful + // matches it, and the mistake lands on the unsafe side - a shipped build + // classified as debuggable. + NSData* startMarker = [@"" dataUsingEncoding:NSUTF8StringEncoding]; + NSRange whole = NSMakeRange(0, [data length]); + NSRange start = [data rangeOfData:startMarker options:0 range:whole]; + if(start.location != NSNotFound) { + NSRange rest = NSMakeRange(start.location, [data length] - start.location); + NSRange end = [data rangeOfData:endMarker options:0 range:rest]; + if(end.location != NSNotFound) { + NSUInteger length = end.location + end.length - start.location; + NSData* plistData = [data subdataWithRange:NSMakeRange(start.location, length)]; + id plist = [NSPropertyListSerialization propertyListWithData:plistData + options:NSPropertyListImmutable + format:NULL + error:NULL]; + if([plist isKindOfClass:[NSDictionary class]]) { + id entitlements = [(NSDictionary*)plist objectForKey:@"Entitlements"]; + if([entitlements isKindOfClass:[NSDictionary class]]) { + id allowed = [(NSDictionary*)entitlements objectForKey:@"get-task-allow"]; + if([allowed isKindOfClass:[NSNumber class]]) { + result = [(NSNumber*)allowed boolValue] ? JAVA_TRUE : JAVA_FALSE; + } + } } } - [text release]; } } } POOL_END(); + // Every path that could not read an answer leaves result JAVA_FALSE: an unreadable or + // unexpected profile means "treat this as a release build", which withholds the + // facility rather than exposing it. return result; #endif } From 2217b7e946b3f6402de42561200cce1f9aa41534 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 26 Jul 2026 18:46:14 +0300 Subject: [PATCH 11/24] Make stopping a listener actually stop it, and stop overclaiming on desktop Two review findings. stop() set a flag and nothing else, so the listener thread stayed parked in accept until some client happened to connect. It then lingered for the life of the process, and because the ports cache one server socket per port, a listener restarted on the same port shared it -- letting the abandoned thread win the race for the first connection and drop it, which is the reconnect that matters most. Re-checking the flag after accept, as I did earlier, addressed the symptom and left the thread parked. Stopping now closes the listening socket, which is what brings accept back. New CodenameOneImplementation.stopListeningSocket(port, loopbackOnly), a no-op by default so a port that does not implement it keeps the old behaviour. JavaSE and Android close and forget the cached socket, so a later listener binds a fresh one; iOS closes the cached loopback descriptor. Separately, Display.isDebuggableBuild() was documented as covering "the simulator and desktop tooling" while JavaSE returns true unconditionally -- so a desktop app packaged for distribution reports a development build, and the doc did not say so. The doc was the defect: JavaSE genuinely cannot tell a packaged desktop app from the tooling, having no debuggable flag or provisioning profile to read, and reporting otherwise would take the MCP menu away from every desktop tool. Now stated plainly per port, in the javadoc, in the port itself and in the guide, including that a desktop app must supply its own signal. Verified in the built artifacts of all three ports, with the iOS sources clang checked against the device and simulator SDKs. 37 MCP tests including a new one asserting close() stops the right listener rather than only flagging it. Co-Authored-By: Claude Opus 5 (1M context) --- .../impl/CodenameOneImplementation.java | 20 +++++++++- CodenameOne/src/com/codename1/io/Socket.java | 6 +++ CodenameOne/src/com/codename1/ui/Display.java | 26 ++++++++++--- .../impl/android/AndroidImplementation.java | 22 +++++++++++ .../com/codename1/impl/javase/JavaSEPort.java | 39 ++++++++++++++++--- Ports/iOSPort/nativeSources/IOSNative.m | 6 +++ Ports/iOSPort/nativeSources/SocketImpl.h | 3 ++ Ports/iOSPort/nativeSources/SocketImpl.m | 17 ++++++++ .../codename1/impl/ios/IOSImplementation.java | 7 ++++ .../src/com/codename1/impl/ios/IOSNative.java | 1 + .../developer-guide/MCP-Headless-API.asciidoc | 4 +- .../mcp/MCPLoopbackTransportOpenTest.java | 19 +++++++++ .../TestCodenameOneImplementation.java | 16 ++++++++ 13 files changed, 172 insertions(+), 14 deletions(-) diff --git a/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java b/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java index 9566424bc85..1b7c49de0a9 100644 --- a/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java +++ b/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java @@ -9931,9 +9931,27 @@ public boolean isLoopbackServerSocketAvailable() { return false; } + /// Closes the listening socket for the given port so a thread parked in accept comes + /// back, which is what makes stopping a listener actually stop it. Setting a flag + /// alone leaves that thread blocked until some client happens to connect, and that + /// late connection is then served or dropped by a listener the caller has abandoned. + /// + /// A port that does not implement this keeps the older behaviour, where stopping takes + /// effect on the next accept. + /// + /// #### Parameters + /// + /// - `port`: the port that was being listened on + /// + /// - `loopbackOnly`: true if the listener was created by [#listenSocketLoopback(int)] + public void stopListeningSocket(int port, boolean loopbackOnly) { + } + /// Whether this build is a development build rather than a release build headed for /// an app store: a debuggable Android package, a development provisioned iOS build, - /// or the simulator and desktop tooling. + /// or a JavaSE process, which covers the simulator, the designer, the desktop tooling + /// and a packaged desktop application alike - see [#isDebuggableBuild()] on the JavaSE + /// port for why it cannot tell them apart. /// /// Facilities that are appropriate while developing but not in a shipped app can gate /// themselves on this. A port that cannot tell should leave the default in place: diff --git a/CodenameOne/src/com/codename1/io/Socket.java b/CodenameOne/src/com/codename1/io/Socket.java index 8afd51d55cc..98e1bff203f 100644 --- a/CodenameOne/src/com/codename1/io/Socket.java +++ b/CodenameOne/src/com/codename1/io/Socket.java @@ -271,6 +271,12 @@ public void run() { @Override public void stop() { stopped = true; + // Closing the listening socket is what brings the thread back out of + // accept. Without it the flag is only noticed the next time a client + // connects, so the thread lingers, and a listener restarted on the same + // port shares the cached socket -- letting this abandoned thread win the + // race for the first connection and drop it. + Util.getImplementation().stopListeningSocket(port, loopbackOnly); } } diff --git a/CodenameOne/src/com/codename1/ui/Display.java b/CodenameOne/src/com/codename1/ui/Display.java index 5fc2ceed4b1..35a1a20f546 100644 --- a/CodenameOne/src/com/codename1/ui/Display.java +++ b/CodenameOne/src/com/codename1/ui/Display.java @@ -6696,13 +6696,27 @@ public boolean isSimulator() { } /// Whether this build is a development build rather than a release build headed for - /// an app store: a debuggable Android package, a development provisioned iOS build, - /// or the simulator and desktop tooling. + /// an app store. This is broader than [#isSimulator()], which is true only in the + /// simulator. Use it to gate a facility that belongs in a build you are working on but + /// not in one a user installs. /// - /// This is broader than [#isSimulator()], which is true only in the simulator. Use it - /// to gate a facility that belongs in a build you are working on but not in one a user - /// installs. A port that cannot tell reports a release build, so the answer errs - /// towards withholding the facility. + /// What each port reports: + /// + /// - Android: true when the package carries the debuggable flag, which a debug build + /// sets and a release build clears. + /// + /// - iOS: true when the provisioning profile grants get-task-allow, the entitlement + /// that permits a debugger to attach. Development and ad-hoc profiles carry it; App + /// Store and enterprise profiles do not. + /// + /// - JavaSE: ALWAYS true. That port runs the simulator, the designer and the desktop + /// tooling, and it cannot distinguish those from a desktop application packaged for + /// distribution, so a packaged desktop app also reports true. Do not rely on this + /// method alone to withhold something from a shipped DESKTOP build; combine it with + /// your own signal there. + /// + /// - Any other port: false, because it cannot tell. The answer errs towards treating a + /// build as a release and withholding the facility. /// /// #### Returns /// diff --git a/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java b/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java index 5d4a6a667ab..765e96fe417 100644 --- a/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java +++ b/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java @@ -11782,6 +11782,23 @@ public synchronized ServerSocket get(int port, boolean loopbackOnly) throws IOEx } return sock; } + + /** + * Closes and forgets the socket, so a thread blocked in accept returns and a + * later listener on this port binds a fresh one rather than sharing this. + */ + public synchronized void close(int port, boolean loopbackOnly) { + Map cache = loopbackOnly ? loopbackSocks : socks; + ServerSocket sock = cache.remove(Integer.valueOf(port)); + if (sock != null) { + try { + sock.close(); + } catch (IOException ignored) { + // best effort: the point is to unblock accept, and a socket that + // cannot be closed is already unusable + } + } + } } @@ -11965,6 +11982,11 @@ public Object listenSocketLoopback(int port) { return new SocketImpl().listen(port, true); } + @Override + public void stopListeningSocket(int port, boolean loopbackOnly) { + getServerSockets().close(port, loopbackOnly); + } + /** * A debuggable package is one built for development: the flag is set by the * build for a debug variant and cleared for a release variant, so this reads the diff --git a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java index 80df9157f72..6d59b202dbb 100644 --- a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java +++ b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java @@ -9724,11 +9724,18 @@ public boolean isSimulator() { return designMode || getSkin() != null || widthLabel != null; } - /// The JavaSE port is the development environment: the simulator, the designer and the - /// desktop tooling all run on it. A desktop application packaged for distribution runs - /// on it too, which is why this is the port where the answer is a judgement rather than - /// a fact read off the build - and the judgement matches the historical behaviour of - /// the tooling that already serves agents from here. + /// Always true, and deliberately so, but it is the weakest answer of any port. + /// + /// This port is the development environment - the simulator, the designer and the + /// desktop tooling all run on it, and that tooling already exposes itself to agents + /// from here. A desktop application packaged for distribution ALSO runs on it, and + /// nothing in the process distinguishes the two: there is no debuggable flag and no + /// provisioning profile to read, as there is on Android and iOS. Reporting a release + /// build here would take the MCP menu away from every desktop tool, which is a shipped + /// feature, so the port answers true and a packaged desktop app is not gated by it. + /// + /// A desktop application that needs to withhold something from its own release build + /// has to supply that signal itself. @Override public boolean isDebuggableBuild() { return true; @@ -17155,6 +17162,21 @@ public synchronized ServerSocket get(int port, boolean loopbackOnly) throws IOEx } return sock; } + + /// Closes and forgets the socket, so a thread blocked in accept returns and a + /// later listener on this port binds a fresh one rather than sharing this. + public synchronized void close(int port, boolean loopbackOnly) { + Map cache = loopbackOnly ? loopbackSocks : socks; + ServerSocket sock = cache.remove(Integer.valueOf(port)); + if (sock != null) { + try { + sock.close(); + } catch (IOException ignored) { + // best effort: the point is to unblock accept, and a socket that + // cannot be closed is already unusable + } + } + } } class SocketImpl { @@ -17333,7 +17355,12 @@ public boolean isLoopbackServerSocketAvailable() { public Object listenSocketLoopback(int port) { return new SocketImpl().listen(port, true); } - + + @Override + public void stopListeningSocket(int port, boolean loopbackOnly) { + getServerSockets().close(port, loopbackOnly); + } + @Override public String getHostOrIP() { try { diff --git a/Ports/iOSPort/nativeSources/IOSNative.m b/Ports/iOSPort/nativeSources/IOSNative.m index 12e9f1c3b57..21449125599 100644 --- a/Ports/iOSPort/nativeSources/IOSNative.m +++ b/Ports/iOSPort/nativeSources/IOSNative.m @@ -10859,6 +10859,12 @@ JAVA_LONG com_codename1_impl_ios_IOSNative_listenSocketLoopback___int(CN1_THREAD return (JAVA_LONG)JAVA_NULL; } +void com_codename1_impl_ios_IOSNative_stopListeningSocket___int(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject, JAVA_INT port) { + POOL_BEGIN(); + [SocketImpl closeLoopbackListenerForPort:port]; + POOL_END(); +} + JAVA_BOOLEAN com_codename1_impl_ios_IOSNative_isDebuggableBuild___R_boolean(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject) { #if TARGET_OS_SIMULATOR // Nothing ships from the simulator, so it is always a development build. It also has diff --git a/Ports/iOSPort/nativeSources/SocketImpl.h b/Ports/iOSPort/nativeSources/SocketImpl.h index a397eeeac27..dfba870fa09 100644 --- a/Ports/iOSPort/nativeSources/SocketImpl.h +++ b/Ports/iOSPort/nativeSources/SocketImpl.h @@ -44,6 +44,9 @@ -(void)disconnect; -(BOOL)listen:(int)param; -(BOOL)listenLoopback:(int)param; +/// Closes the loopback listening socket for a port, bringing a thread blocked in accept +/// back out instead of leaving it waiting for a client nobody wants any more. ++(void)closeLoopbackListenerForPort:(int)port; -(BOOL)isConnected; -(int)getErrorCode; -(BOOL)isSupported; diff --git a/Ports/iOSPort/nativeSources/SocketImpl.m b/Ports/iOSPort/nativeSources/SocketImpl.m index b987dc9906a..a2576e71da4 100644 --- a/Ports/iOSPort/nativeSources/SocketImpl.m +++ b/Ports/iOSPort/nativeSources/SocketImpl.m @@ -292,6 +292,23 @@ +(int)loopbackListenerForPort:(int)port errorOut:(NSString**)errorOut { } } +/// Closes the listening socket for a port and forgets it, so a thread blocked in accept +/// returns instead of waiting for a client that the caller no longer wants, and a later +/// listener on the same port binds a fresh descriptor. ++(void)closeLoopbackListenerForPort:(int)port { + @synchronized([SocketImpl class]) { + if(_cn1LoopbackListeners == nil) { + return; + } + NSNumber* key = [NSNumber numberWithInt:port]; + NSNumber* existing = [_cn1LoopbackListeners objectForKey:key]; + if(existing != nil) { + close([existing intValue]); + [_cn1LoopbackListeners removeObjectForKey:key]; + } + } +} + -(BOOL)listenLoopback:(int)param{ NSString* bindError = nil; int listenFd = [SocketImpl loopbackListenerForPort:param errorOut:&bindError]; diff --git a/Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java b/Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java index cf11fbd5763..3a4846a86e0 100644 --- a/Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java +++ b/Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java @@ -11858,6 +11858,13 @@ public boolean isDebuggableBuild() { return nativeInstance.isDebuggableBuild(); } + @Override + public void stopListeningSocket(int port, boolean loopbackOnly) { + if(loopbackOnly) { + nativeInstance.stopListeningSocket(port); + } + } + @Override public Object listenSocketLoopback(int port) { long peer = nativeInstance.listenSocketLoopback(port); diff --git a/Ports/iOSPort/src/com/codename1/impl/ios/IOSNative.java b/Ports/iOSPort/src/com/codename1/impl/ios/IOSNative.java index dbbbb38eb73..b8477540459 100644 --- a/Ports/iOSPort/src/com/codename1/impl/ios/IOSNative.java +++ b/Ports/iOSPort/src/com/codename1/impl/ios/IOSNative.java @@ -799,6 +799,7 @@ native void gl3dDrawArrays(long contextPeer, long pipelinePeer, long vboPeer, in public native long connectSocket(String host, int port, int connectTimeout); public native long listenSocketLoopback(int port); public native boolean isDebuggableBuild(); + public native void stopListeningSocket(int port); public native String getHostOrIP(); public native void disconnectSocket(long socket); public native boolean isSocketConnected(long socket); diff --git a/docs/developer-guide/MCP-Headless-API.asciidoc b/docs/developer-guide/MCP-Headless-API.asciidoc index 8dae83896b6..79acd9ac230 100644 --- a/docs/developer-guide/MCP-Headless-API.asciidoc +++ b/docs/developer-guide/MCP-Headless-API.asciidoc @@ -41,7 +41,9 @@ The socket server refuses to bind on a release build and throws an `IllegalState The reason is what an attached agent can do. It reads the screen and drives the user interface, and the loopback interface is shared by everything on the device rather than being private to one application. On a phone that means any other installed application can connect to the port and drive yours. That's a reasonable trade while you're developing, because it's how an agent attaches to a running build, and a poor one in an application a user installs. -A development build is a debuggable Android package, a development provisioned iOS build, or the simulator and the desktop tooling. Anything else counts as a release build, so a port that can't tell the difference withholds the server rather than exposing it. +A development build is a debuggable Android package or a development provisioned iOS build. Any other target counts as a release build, so a port that can't tell the difference withholds the server rather than exposing it. + +The JavaSE port is the exception, and it's worth understanding before you rely on this. That port runs the simulator, the designer and the desktop tooling, and it can't distinguish those from a desktop application packaged for distribution: there's no debuggable flag and no provisioning profile to read. It reports a development build in every case, because reporting otherwise would take the MCP menu away from every desktop tool. A packaged desktop application is therefore not gated by this, and one that needs to withhold the server from its own release build has to supply that signal itself. Nothing binds a port unless the application calls a starter, and an application that never calls one has no server at all. The gate matters for the case where a starter is left in the code by accident, which is the way a debugging facility usually reaches production. diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackTransportOpenTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackTransportOpenTest.java index 3e91993c537..57fe6ff99de 100644 --- a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackTransportOpenTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackTransportOpenTest.java @@ -76,4 +76,23 @@ void aFailedListenNeitherStrandsTheRegistrationNorEscapesAsRuntime() throws Exce second = new MCPLoopbackSocketTransport(47882); second.open(); } + + @Test + void closingTheTransportStopsTheListenerRatherThanOnlyFlaggingIt() throws Exception { + // Setting a flag leaves the listener thread parked in accept until some client + // happens to connect. It then lingers, and if a listener is restarted on the same + // port they share the cached socket - so the abandoned thread can win the race for + // the first connection and drop it. Stopping has to close the listening socket. + implementation.setServerSocketAvailable(true); + first = new MCPLoopbackSocketTransport(47883); + first.open(); + assertTrue(implementation.getStoppedListeners().isEmpty(), + "nothing should be stopped while the transport is open"); + + first.close(); + first = null; + assertTrue(implementation.getStoppedListeners().contains("47883:true"), + "close must stop the loopback listener on its own port, got: " + + implementation.getStoppedListeners()); + } } diff --git a/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java b/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java index 34db0c1c6af..f3e98a602cd 100644 --- a/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java +++ b/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java @@ -1282,6 +1282,7 @@ public void reset() { socketAvailable = true; serverSocketAvailable = false; loopbackSupportedOnlyOnFirstQuery = false; + stoppedListeners.clear(); loopbackQueried = false; debuggableBuild = true; appHomePath = "file://app/"; @@ -2791,6 +2792,21 @@ public Object listenSocket(int port) { } } + private final java.util.List stoppedListeners = + java.util.Collections.synchronizedList(new java.util.ArrayList()); + + /// Records that a listener was asked to stop, as "port:loopbackOnly". Real ports close + /// the listening socket here; there is no socket to close in these tests, so what is + /// observable is that the call was made at all, and for the right listener. + @Override + public void stopListeningSocket(int port, boolean loopbackOnly) { + stoppedListeners.add(port + ":" + loopbackOnly); + } + + public java.util.List getStoppedListeners() { + return stoppedListeners; + } + private boolean loopbackSupportedOnlyOnFirstQuery; private boolean loopbackQueried; From 8c89c8d679708d15fb28339401643137bc8421f5 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 26 Jul 2026 20:58:20 +0300 Subject: [PATCH 12/24] Two more documentation overclaims about what counts as development Both are mine, and both said something narrower than the code does. Display.isDebuggableBuild() described isSimulator() as "true only in the simulator". On JavaSE it is designMode || skin != null || simulator chrome != null, so it is also true in the designer. Reworded to name the simulator and the designer, and to say it is false on a device however that build was signed -- which is the distinction a caller is actually choosing between. MCP.setAllowOnReleaseBuilds() still described a development build as "the simulator and desktop tooling", the same overclaim I corrected in Display and the guide last commit but missed here. This is the doc a developer reads while deciding whether to touch the override, so it is the worst place to leave it. It now says plainly that the JavaSE port reports a development build in every case and that this gate does NOT protect a shipped desktop build. No behaviour change. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/mcp/MCP.java | 14 ++++++++++---- CodenameOne/src/com/codename1/ui/Display.java | 5 +++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/CodenameOne/src/com/codename1/mcp/MCP.java b/CodenameOne/src/com/codename1/mcp/MCP.java index 9397bfcb046..188ffce7a79 100644 --- a/CodenameOne/src/com/codename1/mcp/MCP.java +++ b/CodenameOne/src/com/codename1/mcp/MCP.java @@ -159,10 +159,16 @@ public static synchronized MCPServer startSocketServer(int port) { /// trade while developing - it is how an agent attaches to a running app - and a poor /// one in an app a user installs. /// - /// A development build is detected as a debuggable Android package, a development - /// provisioned iOS build, or the simulator and desktop tooling. Anything else is - /// treated as a release build, so a port that cannot tell withholds the server rather - /// than exposing it. + /// A development build is detected as a debuggable Android package or a development + /// provisioned iOS build. Any other target is treated as a release build, so a port + /// that cannot tell withholds the server rather than exposing it. + /// + /// The JavaSE port is the exception, and it matters here: it reports a development + /// build in every case, because it cannot distinguish the simulator, the designer and + /// the desktop tooling from a desktop application packaged for distribution. So this + /// gate does NOT protect a shipped desktop build, and a desktop application that wants + /// the server withheld from its own release has to decide that for itself. See + /// [com.codename1.ui.Display#isDebuggableBuild()]. /// /// Set this only for a build that ships to a controlled fleet - a kiosk, a test lab, /// an enterprise deployment - where the device itself is trusted. diff --git a/CodenameOne/src/com/codename1/ui/Display.java b/CodenameOne/src/com/codename1/ui/Display.java index 35a1a20f546..9618a61c2f8 100644 --- a/CodenameOne/src/com/codename1/ui/Display.java +++ b/CodenameOne/src/com/codename1/ui/Display.java @@ -6696,8 +6696,9 @@ public boolean isSimulator() { } /// Whether this build is a development build rather than a release build headed for - /// an app store. This is broader than [#isSimulator()], which is true only in the - /// simulator. Use it to gate a facility that belongs in a build you are working on but + /// an app store. This is broader than [#isSimulator()], which reports the JavaSE + /// simulator and designer specifically and is false on a device however the build was + /// signed. Use it to gate a facility that belongs in a build you are working on but /// not in one a user installs. /// /// What each port reports: From 9e312fd96871ec82930841eb0d206e2af3ee686c Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 27 Jul 2026 00:49:13 +0300 Subject: [PATCH 13/24] Publish the listener stop flag across threads with an AtomicBoolean stop() wrote `stopped` on the caller's thread and the accept loop read it on another, with no memory barrier between them. Closing the listening socket made that worse rather than better: the accept returns, the loop re-tests a flag it may not have observed, and the port's cache hands out a FRESH socket for the next call because the old one is closed. So the listener rebinds and carries on, which is the opposite of what stop() was asked to do, with spurious connectionError callbacks along the way. AtomicBoolean rather than volatile, per review. It is already used elsewhere in core and the ParparVM runtime implements it properly. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/io/Socket.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/CodenameOne/src/com/codename1/io/Socket.java b/CodenameOne/src/com/codename1/io/Socket.java index 98e1bff203f..877513ed805 100644 --- a/CodenameOne/src/com/codename1/io/Socket.java +++ b/CodenameOne/src/com/codename1/io/Socket.java @@ -29,6 +29,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.concurrent.atomic.AtomicBoolean; /// Class implementing the socket API /// @@ -222,16 +223,21 @@ public static StopListening listenLoopback(final int port, final Class scClass) private static StopListening listenImpl(final int port, final Class scClass, final boolean loopbackOnly) { class Listener implements StopListening, Runnable { - private boolean stopped; + /// Written by stop() on the caller's thread and read by the accept loop on + /// another, so it needs a memory barrier rather than a plain field. Without + /// one the loop can miss the write, and since closing the listening socket + /// makes the port's cache hand out a FRESH socket on the next call, the + /// listener would quietly resurrect itself instead of stopping. + private final AtomicBoolean stopped = new AtomicBoolean(); @Override public void run() { try { - while (!stopped) { + while (!stopped.get()) { final Object connection = loopbackOnly ? Util.getImplementation().listenSocketLoopback(port) : Util.getImplementation().listenSocket(port); - if (stopped) { + if (stopped.get()) { // stop() was called while this thread sat inside accept. The // connection that unblocked it belongs to a listener the caller // has already abandoned, so hand it back rather than serving it. @@ -270,7 +276,7 @@ public void run() { @Override public void stop() { - stopped = true; + stopped.set(true); // Closing the listening socket is what brings the thread back out of // accept. Without it the flag is only noticed the next time a client // connects, so the thread lingers, and a listener restarted on the same From 2ac902d83e87af6b4aef8d035ed18ca4a2b210b5 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 27 Jul 2026 03:40:02 +0300 Subject: [PATCH 14/24] Bound the size of an incoming MCP frame readLine accumulated into an unbounded buffer, so a client that sent no delimiter could grow it until the process ran out of memory. That is reachable: the channel is loopback, and on a device any other installed app can open it. An oversized frame is now treated as a disconnect, the same as a truncated one, so the server stays bound for the next session rather than dying. The ceiling is 8MB, far above anything legitimate -- what arrives here are requests, tool calls and UI actions, while the bulky payloads such as screenshots travel the other way. The check sits at the head of the read loop rather than at each write site, so both the payload byte and the held-back carriage return are covered by one test. The test models the attack rather than a large string: a stream that returns a byte forever and never delimits. It asserts that the reader STOPS asking for bytes, which is what distinguishes a ceiling from none -- the thread stays alive either way. Control-tested with the ceiling disabled, where it consumed 179 million bytes in under a second and was still climbing. Co-Authored-By: Claude Opus 5 (1M context) --- .../mcp/MCPLoopbackSocketTransport.java | 13 ++++++ .../mcp/MCPLoopbackSocketTransportTest.java | 43 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java index 94e7779261c..3e4de167842 100644 --- a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java +++ b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java @@ -48,6 +48,12 @@ public final class MCPLoopbackSocketTransport implements MCPTransport { /// suffices because MCP runs one server per process. private static MCPLoopbackSocketTransport active; + /// Ceiling on one incoming frame. What arrives here are requests - tool calls and UI + /// actions - while the bulky payloads, screenshots among them, travel the other way, + /// so this sits far above any legitimate message. It exists to bound what an ill + /// behaved or hostile client can make the server allocate. + private static final int MAX_FRAME_BYTES = 8 * 1024 * 1024; + private final int port; private final Object lock = new Object(); private Socket.StopListening listening; @@ -192,6 +198,13 @@ private String readLine(InputStream stream) throws IOException { // a payload that happened to contain one. boolean pendingCarriageReturn = false; while (true) { + if (buffer.size() >= MAX_FRAME_BYTES) { + // A client that sends no delimiter would otherwise grow this buffer until + // the process runs out of memory, and on a device any other app installed + // alongside can open this connection. Treat it as a disconnect, the same + // as a truncated frame, so the server stays bound for the next session. + return null; + } int b; try { b = stream.read(); diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java index 257677f9fdb..045fbbe76ce 100644 --- a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java @@ -31,6 +31,7 @@ import java.io.InputStream; import java.io.PipedInputStream; import java.io.PipedOutputStream; +import java.util.concurrent.atomic.AtomicLong; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -151,6 +152,48 @@ void aDisconnectedClientDoesNotEndTheServer() throws Exception { assertEquals("{\"id\":9}", out[0], "the reconnecting agent's message should arrive"); } + @Test + void aClientThatNeverDelimitsItsFrameCannotExhaustMemory() throws Exception { + // A stream that never ends and never delimits: without a ceiling on the frame the + // reader would accumulate until the process died, and on a device any other + // installed app can open this connection. It must be dropped like any other bad + // client, leaving the server available for the next session. + final MCPLoopbackSocketTransport t = new MCPLoopbackSocketTransport(47811); + transport = t; + // Counting what the client is asked for is what distinguishes a ceiling from no + // ceiling here: the thread stays alive either way, but an uncapped reader never + // stops asking for more. + final AtomicLong served = new AtomicLong(); + t.attach(new InputStream() { + @Override + public int read() { + served.incrementAndGet(); + return 'x'; + } + }, new ByteArrayOutputStream()); + + final String[] out = new String[1]; + Thread reader = new Thread(() -> { + try { + out[0] = t.readMessage(); + } catch (IOException ignored) { + // surfaces as a null message below + } + }); + reader.start(); + Thread.sleep(500); + long afterSettling = served.get(); + Thread.sleep(250); + assertEquals(afterSettling, served.get(), + "reading must stop at the frame ceiling; it was still consuming bytes"); + + t.attach(new ByteArrayInputStream("{\"id\":11}\n".getBytes("UTF-8")), + new ByteArrayOutputStream()); + reader.join(5000); + assertEquals("{\"id\":11}", out[0], + "the next client's message should arrive normally"); + } + @Test void aFrameCutShortByADisconnectIsNotDeliveredAsAMessage() throws Exception { // A client that dies mid-frame leaves bytes with no delimiter. Delivering those From 79c91cc846bf0e37003c433a4971fa5536283f1d Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 27 Jul 2026 06:22:45 +0300 Subject: [PATCH 15/24] Stop logging a stack trace for a deliberate listener stop Closing the listening socket to unblock accept, which is how stop() now works, makes accept throw as a normal part of shutting down. Both the JavaSE and Android listen paths printed a full stack trace for any exception there, so every intentional stop wrote an alarming fake failure into the log -- and MCP stops its listener on every close. They now print only when the socket was not closed, which is the case that represents a real failure. The server socket reference is hoisted out of the try so the catch can tell the difference; if the lookup itself failed there is no socket to inspect and the trace is printed as before. Also correct the startSocketServer javadoc, which still said a platform transport factory was required and registered by the JavaSE port. That stopped being true when the portable fallback landed: a port without its own transport now binds through com.codename1.io.Socket, which is the whole reason this works on a device. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/mcp/MCP.java | 6 +++++- .../codename1/impl/android/AndroidImplementation.java | 11 +++++++++-- .../src/com/codename1/impl/javase/JavaSEPort.java | 11 +++++++++-- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/CodenameOne/src/com/codename1/mcp/MCP.java b/CodenameOne/src/com/codename1/mcp/MCP.java index 188ffce7a79..7f3265e42c2 100644 --- a/CodenameOne/src/com/codename1/mcp/MCP.java +++ b/CodenameOne/src/com/codename1/mcp/MCP.java @@ -129,7 +129,11 @@ public static synchronized MCPServer startStdioServer() { } /// Starts a loopback socket server so an agent can attach to this running process. - /// Requires a platform socket transport factory (registered by the JavaSE port). + /// + /// Uses the transport the port registered, if it has one - the JavaSE port registers a + /// java.net implementation. Any other port falls back to the portable transport, which + /// binds through [com.codename1.io.Socket], so this works on a device too. It fails, + /// on this thread, when the platform cannot bind a loopback server socket at all. /// /// Refuses to bind on a RELEASE build, throwing [IllegalStateException] - see /// [#setAllowOnReleaseBuilds(boolean)] for why, and for the deliberate override. diff --git a/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java b/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java index 765e96fe417..deaa5a48f3f 100644 --- a/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java +++ b/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java @@ -11929,15 +11929,22 @@ public Object listen(int param) { } public Object listen(int param, boolean loopbackOnly) { + ServerSocket serverSocketInstance = null; try { - ServerSocket serverSocketInstance = getServerSockets().get(param, loopbackOnly); + serverSocketInstance = getServerSockets().get(param, loopbackOnly); socketInstance = serverSocketInstance.accept(); SocketImpl si = new SocketImpl(); si.socketInstance = socketInstance; return si; } catch(Exception err) { errorMessage = err.toString(); - err.printStackTrace(); + // A closed socket here is the deliberate stop path: stopping a + // listener closes it precisely to bring this accept back. Printing a + // stack trace for that would put an alarming fake failure in the log + // every time a listener is stopped. + if(serverSocketInstance == null || !serverSocketInstance.isClosed()) { + err.printStackTrace(); + } return null; } } diff --git a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java index 6d59b202dbb..aee666fc366 100644 --- a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java +++ b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java @@ -17311,13 +17311,20 @@ public Object listen(int param) { } public Object listen(int param, boolean loopbackOnly) { + ServerSocket serverSocketInstance = null; try { - ServerSocket serverSocketInstance = getServerSockets().get(param, loopbackOnly); + serverSocketInstance = getServerSockets().get(param, loopbackOnly); socketInstance = serverSocketInstance.accept(); return this; } catch(Exception err) { errorMessage = err.toString(); - err.printStackTrace(); + // A closed socket here is the deliberate stop path: stopping a listener + // closes it precisely to bring this accept back. Printing a stack trace + // for that would put an alarming fake failure in the log every time a + // listener is stopped. + if(serverSocketInstance == null || !serverSocketInstance.isClosed()) { + err.printStackTrace(); + } return null; } } From 7812b51b2194f6adad34fd1671c3ae5557c332b3 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 27 Jul 2026 09:45:57 +0300 Subject: [PATCH 16/24] Review round: dropped clients, loopback family, EINTR, frame boundary, flaky tests attach() documented that the previous client is dropped but only overwrote the field. A reader parked in read() on the old stream is not watching that field, so a client that held its connection open and sent nothing kept the server serving a session that had already been replaced. The old streams are now closed, outside the lock, since closing is what wakes that reader. Loopback binds are named as 127.0.0.1 rather than taken from InetAddress.getLoopbackAddress(), which answers ::1 where the runtime prefers IPv6. A client connecting to 127.0.0.1 -- what adb forward and attaching agents do, and what the iOS port already binds -- would have found nothing listening while the server reported that it had started. recv and send retry on EINTR instead of treating it as a disconnect. This process installs signal handlers, so a signal arriving mid-call is ordinary here and dropping a healthy connection over one would be wrong. accept distinguishes the errnos that mean "you were asked to stop" (EBADF, EINVAL, EINTR, which is how closing the descriptor is delivered) from a real failure, so a deliberate shutdown no longer looks like a fault. The frame ceiling was off by one: a payload of exactly MAX_FRAME_BYTES was rejected before its delimiter could be read, making the largest permitted message unsendable. The ceiling is on the payload, so the test is now >. Tests: the timing-based sleeps are replaced with waits on observable conditions -- bytes consumed, end of stream reached, reader thread parked in WAITING -- which removes the flakiness and also cut this class from 1.25s to 0.26s. The new boundary test reads on a thread with a deadline: writing it as a direct call made the control run HANG rather than fail, because the regression parks the reader. And the gate test asserts its message is non-null before matching on it. Declined, with reasons in the PR: Class#newInstance cannot be replaced with getDeclaredConstructor().newInstance() because ParparVM's Class has no such method and its Constructor has only getName(), so that change would break the iOS build outright. Co-Authored-By: Claude Opus 5 (1M context) --- .../mcp/MCPLoopbackSocketTransport.java | 43 ++++++- .../impl/android/AndroidImplementation.java | 8 +- .../com/codename1/impl/javase/JavaSEPort.java | 8 +- Ports/iOSPort/nativeSources/SocketImpl.m | 33 +++-- .../mcp/MCPLoopbackSocketTransportTest.java | 113 ++++++++++++++++-- .../mcp/MCPReleaseBuildGateTest.java | 7 +- 6 files changed, 188 insertions(+), 24 deletions(-) diff --git a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java index 3e4de167842..da44d94db07 100644 --- a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java +++ b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java @@ -52,7 +52,7 @@ public final class MCPLoopbackSocketTransport implements MCPTransport { /// actions - while the bulky payloads, screenshots among them, travel the other way, /// so this sits far above any legitimate message. It exists to bound what an ill /// behaved or hostile client can make the server allocate. - private static final int MAX_FRAME_BYTES = 8 * 1024 * 1024; + static final int MAX_FRAME_BYTES = 8 * 1024 * 1024; private final int port; private final Object lock = new Object(); @@ -133,11 +133,50 @@ private void clearActiveIfOurs() { /// Called from the connection callback thread when a client attaches. The previous /// client, if any, is dropped: one agent drives at a time. void attach(InputStream is, OutputStream os) { + InputStream previousIn; // NOPMD closed below, deliberately outside the lock + OutputStream previousOut; // NOPMD closed below, outside the lock synchronized (lock) { + previousIn = in; + previousOut = out; in = is; out = os; lock.notifyAll(); } + // Dropping the previous client means closing its streams, not just forgetting + // them. A reader parked in read() on the old stream is not watching this field, + // so a client that holds its connection open and sends nothing would keep the + // server serving a session that has already been replaced. + // + // Outside the lock, because closing is what wakes that reader and it takes the + // lock on its way out. + if (previousIn != is) { // NOPMD identity: the same stream re-attached is not a swap + closeQuietly(previousIn); + } + if (previousOut != os) { // NOPMD identity: as above + closeQuietly(previousOut); + } + } + + /// Closes a dropped client's stream. Failure is uninteresting: the stream is being + /// discarded either way, and closing is best effort by nature. + private static void closeQuietly(InputStream stream) { + if (stream != null) { + try { + stream.close(); + } catch (IOException ignored) { + // discarding this stream regardless + } + } + } + + private static void closeQuietly(OutputStream stream) { + if (stream != null) { + try { + stream.close(); + } catch (IOException ignored) { + // discarding this stream regardless + } + } } /// Called when a client's session ends. @@ -198,7 +237,7 @@ private String readLine(InputStream stream) throws IOException { // a payload that happened to contain one. boolean pendingCarriageReturn = false; while (true) { - if (buffer.size() >= MAX_FRAME_BYTES) { + if (buffer.size() > MAX_FRAME_BYTES) { // A client that sends no delimiter would otherwise grow this buffer until // the process runs out of memory, and on a device any other app installed // alongside can open this connection. Treat it as a disconnect, the same diff --git a/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java b/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java index deaa5a48f3f..d2821da8e12 100644 --- a/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java +++ b/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java @@ -11769,6 +11769,12 @@ public synchronized ServerSocket get(int port) throws IOException { * address must never be handed back to a caller that asked for loopback. * Distinguishing them by sign within one map would collide on port 0, the * ephemeral-port request, where -0 == 0. + * + * The IPv4 loopback is named explicitly rather than taken from + * InetAddress.getLoopbackAddress(), which answers ::1 when the runtime + * prefers IPv6. A client that then connects to 127.0.0.1 - which is what + * adb forward and attaching agents do, and what the iOS port binds - would + * find nothing listening, with the server reporting that it had started. */ public synchronized ServerSocket get(int port, boolean loopbackOnly) throws IOException { Map cache = loopbackOnly ? loopbackSocks : socks; @@ -11776,7 +11782,7 @@ public synchronized ServerSocket get(int port, boolean loopbackOnly) throws IOEx ServerSocket sock = cache.get(key); if (sock == null || sock.isClosed()) { sock = loopbackOnly - ? new ServerSocket(port, 50, InetAddress.getLoopbackAddress()) + ? new ServerSocket(port, 50, InetAddress.getByName("127.0.0.1")) : new ServerSocket(port); cache.put(key, sock); } diff --git a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java index aee666fc366..357cf751e1d 100644 --- a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java +++ b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java @@ -17150,13 +17150,19 @@ public synchronized ServerSocket get(int port) throws IOException { /// SEPARATE maps: a port already bound wildcard must never be handed back to a /// caller that asked for loopback. Distinguishing them by sign within one map /// would collide on port 0, the ephemeral-port request, where -0 == 0. + /// + /// The IPv4 loopback is named explicitly rather than taken from + /// InetAddress.getLoopbackAddress(), which answers ::1 when the runtime prefers + /// IPv6. A client that then connects to 127.0.0.1 - which is what agents and + /// port-forwarding tools do, and what the iOS port binds - would find nothing + /// listening, with the server reporting that it had started. public synchronized ServerSocket get(int port, boolean loopbackOnly) throws IOException { Map cache = loopbackOnly ? loopbackSocks : socks; Integer key = Integer.valueOf(port); ServerSocket sock = cache.get(key); if (sock == null || sock.isClosed()) { sock = loopbackOnly - ? new ServerSocket(port, 50, InetAddress.getLoopbackAddress()) + ? new ServerSocket(port, 50, InetAddress.getByName("127.0.0.1")) : new ServerSocket(port); cache.put(key, sock); } diff --git a/Ports/iOSPort/nativeSources/SocketImpl.m b/Ports/iOSPort/nativeSources/SocketImpl.m index a2576e71da4..82ad439edab 100644 --- a/Ports/iOSPort/nativeSources/SocketImpl.m +++ b/Ports/iOSPort/nativeSources/SocketImpl.m @@ -26,6 +26,7 @@ #include #include #include +#include #include #include "xmlvm.h" #import "CodenameOne_GLViewController.h" @@ -169,9 +170,16 @@ -(NSData*)readFromStream{ if(rawFd >= 0) { // Blocking recv: the caller is a reader thread and expects to wait for data, // and returning nil in a spin would burn the CPU. - _yield(); - ssize_t r = recv(rawFd, buffer, sizeof(buffer), 0); - _resume(); + // + // EINTR is retried rather than treated as a disconnect. This process installs + // signal handlers, so a signal arriving mid-read is an ordinary event here, and + // dropping a healthy connection because of one would be wrong. + ssize_t r; + do { + _yield(); + r = recv(rawFd, buffer, sizeof(buffer), 0); + _resume(); + } while(r < 0 && errno == EINTR); if(r > 0) { return [NSData dataWithBytes:buffer length:r]; } @@ -200,9 +208,12 @@ -(void)writeToStream:(NSData*)param{ // send() blocks once the send buffer fills, so hand the thread back to the VM // across it exactly as the accept and recv paths do; holding it would stall // cooperative scheduling for as long as the peer is slow to drain. - _yield(); - ssize_t w = send(rawFd, bytes, remaining, 0); - _resume(); + ssize_t w; + do { + _yield(); + w = send(rawFd, bytes, remaining, 0); + _resume(); + } while(w < 0 && errno == EINTR); if(w <= 0) { [self closeRawDescriptor]; return; @@ -325,9 +336,17 @@ -(BOOL)listenLoopback:(int)param{ clientFd = accept(listenFd, NULL, NULL); _resume(); if(clientFd < 0) { - errorMessage = @"Accept failed"; + int acceptErrno = errno; errorCode = -1; connected = NO; + // Stopping a listener is delivered by closing its descriptor, so these mean + // "you were asked to stop", not "the accept failed". Reporting them as errors + // would make every deliberate shutdown look like a fault to the Java side. + if(acceptErrno == EBADF || acceptErrno == EINVAL || acceptErrno == EINTR) { + errorMessage = nil; + } else { + errorMessage = @"Accept failed"; + } return NO; } diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java index 045fbbe76ce..446b05cb2ee 100644 --- a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java @@ -34,6 +34,7 @@ import java.util.concurrent.atomic.AtomicLong; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -59,6 +60,21 @@ void closeTransport() { } } + /// Waits for a condition instead of sleeping for a guessed interval. A fixed sleep + /// either wastes time or, on a loaded machine, fires before the thread it is waiting + /// on has got anywhere - which is how timing-based tests turn flaky. + private static void await(String what, java.util.concurrent.Callable condition) + throws Exception { + long deadline = System.currentTimeMillis() + 10000; + while (System.currentTimeMillis() < deadline) { + if (condition.call()) { + return; + } + Thread.sleep(5); + } + fail("timed out waiting for " + what); + } + private MCPLoopbackSocketTransport attached(String clientBytes) throws IOException { transport = new MCPLoopbackSocketTransport(47811); transport.attach(new ByteArrayInputStream(clientBytes.getBytes("UTF-8")), @@ -132,7 +148,16 @@ void aDisconnectedClientDoesNotEndTheServer() throws Exception { // instead of reporting end-of-transport, so reconnects keep working. final MCPLoopbackSocketTransport t = new MCPLoopbackSocketTransport(47811); transport = t; - t.attach(new ByteArrayInputStream(new byte[0]), new ByteArrayOutputStream()); + // Counts the end-of-stream read, so the test can wait for the reader to have SEEN + // the disconnect rather than sleeping and hoping. + final AtomicLong endOfStreamReads = new AtomicLong(); + t.attach(new InputStream() { + @Override + public int read() { + endOfStreamReads.incrementAndGet(); + return -1; + } + }, new ByteArrayOutputStream()); final String[] out = new String[1]; Thread reader = new Thread(() -> { @@ -143,7 +168,7 @@ void aDisconnectedClientDoesNotEndTheServer() throws Exception { } }); reader.start(); - Thread.sleep(150); + await("the reader to consume the disconnect", () -> endOfStreamReads.get() > 0); assertTrue(reader.isAlive(), "the reader should be waiting for the next client"); t.attach(new ByteArrayInputStream("{\"id\":9}\n".getBytes("UTF-8")), @@ -181,11 +206,13 @@ public int read() { } }); reader.start(); - Thread.sleep(500); - long afterSettling = served.get(); - Thread.sleep(250); - assertEquals(afterSettling, served.get(), - "reading must stop at the frame ceiling; it was still consuming bytes"); + // Wait for consumption to stop rather than for a fixed interval: the ceiling is + // what makes it stop, and how long 8MB takes depends on the machine. + await("the reader to stop consuming at the frame ceiling", () -> { + long before = served.get(); + Thread.sleep(50); + return before == served.get(); + }); t.attach(new ByteArrayInputStream("{\"id\":11}\n".getBytes("UTF-8")), new ByteArrayOutputStream()); @@ -194,6 +221,50 @@ public int read() { "the next client's message should arrive normally"); } + @Test + void aFrameOfExactlyTheCeilingIsStillDelivered() throws Exception { + // The ceiling is on the payload, so a frame of exactly that size is legal and its + // delimiter still has to be read. Rejecting at >= would have made the largest + // permitted message unsendable, which is the classic off-by-one on a limit. + final int size = MCPLoopbackSocketTransport.MAX_FRAME_BYTES; + final MCPLoopbackSocketTransport t = new MCPLoopbackSocketTransport(47811); + transport = t; + t.attach(new InputStream() { + private int pos; + + @Override + public int read() { + if (pos < size) { + pos++; + return 'x'; + } + if (pos == size) { + pos++; + return '\n'; + } + return -1; + } + }, new ByteArrayOutputStream()); + + // Read on another thread with a deadline. Should the ceiling ever go back to + // rejecting at exactly the limit, this frame is discarded as a disconnect and the + // reader parks waiting for the next client -- so a direct call here would hang CI + // instead of failing it. + final String[] out = new String[1]; + Thread reader = new Thread(() -> { + try { + out[0] = t.readMessage(); + } catch (IOException ignored) { + // surfaces as a null message below + } + }); + reader.start(); + reader.join(30000); + assertNotNull(out[0], "a frame of exactly the ceiling must be delivered"); + assertEquals(size, out[0].length(), + "the whole payload should arrive, without the delimiter"); + } + @Test void aFrameCutShortByADisconnectIsNotDeliveredAsAMessage() throws Exception { // A client that dies mid-frame leaves bytes with no delimiter. Delivering those @@ -202,8 +273,22 @@ void aFrameCutShortByADisconnectIsNotDeliveredAsAMessage() throws Exception { // stay available, exactly as it does for a clean end of stream. final MCPLoopbackSocketTransport t = new MCPLoopbackSocketTransport(47811); transport = t; - t.attach(new ByteArrayInputStream("{\"id\":1,\"method\":\"init".getBytes("UTF-8")), - new ByteArrayOutputStream()); + // Yields a partial frame then end of stream, counting the EOF so the test can wait + // for the reader to have reached it. + final AtomicLong truncatedReads = new AtomicLong(); + final byte[] partial = "{\"id\":1,\"method\":\"init".getBytes("UTF-8"); + t.attach(new InputStream() { + private int pos; + + @Override + public int read() { + if (pos < partial.length) { + return partial[pos++] & 0xff; + } + truncatedReads.incrementAndGet(); + return -1; + } + }, new ByteArrayOutputStream()); final String[] out = new String[1]; Thread reader = new Thread(() -> { @@ -214,7 +299,7 @@ void aFrameCutShortByADisconnectIsNotDeliveredAsAMessage() throws Exception { } }); reader.start(); - Thread.sleep(150); + await("the reader to consume the truncated frame", () -> truncatedReads.get() > 0); assertTrue(reader.isAlive(), "a truncated frame must leave the reader waiting for the next client, " + "not hand up a partial message"); @@ -232,7 +317,7 @@ void closeUnblocksAPendingRead() throws Exception { transport = t; final String[] out = new String[1]; final boolean[] done = new boolean[1]; - Thread reader = new Thread(() -> { + final Thread reader = new Thread(() -> { try { out[0] = t.readMessage(); } catch (IOException ignored) { @@ -241,7 +326,11 @@ void closeUnblocksAPendingRead() throws Exception { done[0] = true; }); reader.start(); - Thread.sleep(150); + // Park the reader before closing, so this exercises "close wakes a waiting reader" + // rather than "close had already been called". WAITING is precisely the state + // lock.wait() puts it in. + await("the reader to park waiting for a client", + () -> reader.getState() == Thread.State.WAITING); t.close(); reader.join(3000); diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java index 25eb613d42b..bf8fcef6830 100644 --- a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java @@ -57,8 +57,13 @@ void clearOverride() { private String refusal(boolean debuggable) { implementation.setDebuggableBuild(debuggable); implementation.setServerSocketAvailable(true); - return assertThrows(IllegalStateException.class, + String message = assertThrows(IllegalStateException.class, () -> MCP.startSocketServer(47899)).getMessage(); + // getMessage() is allowed to be null; assert it here so a future change that drops + // the message fails as a readable assertion rather than a NullPointerException in + // the caller's contains() check. + assertNotNull(message, "the refusal must explain itself"); + return message; } /// Asserts the gate let the start through, leaving a running server behind. From e902cc53beb7ee0bd4d7603e3d8f194deddd85e8 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 27 Jul 2026 12:47:43 +0300 Subject: [PATCH 17/24] Retry accept on EINTR, back off a failing listener, make the ceiling exact Three from review, one of which corrects my own previous commit. accept now retries on EINTR rather than reporting it. I had listed EINTR among the errnos meaning "you were asked to stop", which was wrong: stopping is delivered by CLOSING the descriptor, and that surfaces as EBADF. EINTR is a spurious wakeup from a signal, and treating it as a shutdown would have dropped a healthy listener on a platform that installs signal handlers. Only EBADF and EINVAL mean stop now, and this matches the retry already used by recv and send. The listener loop pauses before retrying when an accept fails. A failure that recurs -- a port that cannot be bound -- had it spinning at full speed, burning a core and filling the log. Only the failure path waits; a served connection never reaches it. The frame ceiling is enforced where bytes are appended rather than at the top of the loop, so the limit is exact in both directions: a frame of precisely MAX_FRAME_BYTES is accepted and its delimiter read, and one byte beyond is refused before it is buffered. The previous form was correct about memory but allowed the buffer a byte or two past the stated ceiling, which is harder to reason about than it is worth. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/io/Socket.java | 10 ++++++ .../mcp/MCPLoopbackSocketTransport.java | 31 +++++++++++++------ Ports/iOSPort/nativeSources/SocketImpl.m | 14 ++++++--- 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/CodenameOne/src/com/codename1/io/Socket.java b/CodenameOne/src/com/codename1/io/Socket.java index 877513ed805..fdf10500be7 100644 --- a/CodenameOne/src/com/codename1/io/Socket.java +++ b/CodenameOne/src/com/codename1/io/Socket.java @@ -260,6 +260,16 @@ public void run() { }, "Connection " + port).start(); } else { sc.connectionError(Util.getImplementation().getSocketErrorCode(connection), Util.getImplementation().getSocketErrorMessage(connection)); + // A listener whose accept fails immediately and keeps failing -- + // a port that cannot be bound, say -- would otherwise spin at + // full speed, burning a core and filling the log. Pause so a + // persistent failure is slow and visible instead of hot. Only + // the failure path waits; a served connection never gets here. + try { + Thread.sleep(500); + } catch (InterruptedException interrupted) { + // fall through: the loop re-tests stopped straight away + } } } } catch (InstantiationException err) { diff --git a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java index da44d94db07..2186b0fd91f 100644 --- a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java +++ b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java @@ -237,13 +237,6 @@ private String readLine(InputStream stream) throws IOException { // a payload that happened to contain one. boolean pendingCarriageReturn = false; while (true) { - if (buffer.size() > MAX_FRAME_BYTES) { - // A client that sends no delimiter would otherwise grow this buffer until - // the process runs out of memory, and on a device any other app installed - // alongside can open this connection. Treat it as a disconnect, the same - // as a truncated frame, so the server stays bound for the next session. - return null; - } int b; try { b = stream.read(); @@ -263,7 +256,9 @@ private String readLine(InputStream stream) throws IOException { if (b == '\n') { return toUtf8(buffer); // the CR belonged to a CRLF delimiter } - buffer.write('\r'); // it belonged to the payload after all + if (!append(buffer, '\r')) { // it belonged to the payload after all + return null; + } } if (b == '\r') { pendingCarriageReturn = true; @@ -272,8 +267,26 @@ private String readLine(InputStream stream) throws IOException { if (b == '\n') { return toUtf8(buffer); } - buffer.write(b); + if (!append(buffer, b)) { + return null; + } + } + } + + /// Appends one payload byte, refusing once the frame would pass [#MAX_FRAME_BYTES]. + /// Enforced here rather than at the top of the read loop so the ceiling is exact: a + /// frame of precisely the limit is accepted and its delimiter still read, and one byte + /// beyond is refused before it is ever buffered. + /// + /// A refusal is reported to the caller as end of stream. A client sending no delimiter + /// would otherwise grow this buffer until the process ran out of memory, and on a + /// device any other app installed alongside can open this connection. + private static boolean append(ByteArrayOutputStream buffer, int b) { + if (buffer.size() >= MAX_FRAME_BYTES) { + return false; } + buffer.write(b); + return true; } private static String toUtf8(ByteArrayOutputStream buffer) throws IOException { diff --git a/Ports/iOSPort/nativeSources/SocketImpl.m b/Ports/iOSPort/nativeSources/SocketImpl.m index 82ad439edab..97fc8f41cfd 100644 --- a/Ports/iOSPort/nativeSources/SocketImpl.m +++ b/Ports/iOSPort/nativeSources/SocketImpl.m @@ -330,11 +330,15 @@ -(BOOL)listenLoopback:(int)param{ return NO; } - // accept() blocks; hand the VM back this thread while it does. + // accept() blocks; hand the VM back this thread while it does. EINTR is retried + // rather than reported: a signal arriving while parked here is a spurious wakeup, not + // a request to stop. Stopping arrives as a CLOSED descriptor, which is EBADF. int clientFd; - _yield(); - clientFd = accept(listenFd, NULL, NULL); - _resume(); + do { + _yield(); + clientFd = accept(listenFd, NULL, NULL); + _resume(); + } while(clientFd < 0 && errno == EINTR); if(clientFd < 0) { int acceptErrno = errno; errorCode = -1; @@ -342,7 +346,7 @@ -(BOOL)listenLoopback:(int)param{ // Stopping a listener is delivered by closing its descriptor, so these mean // "you were asked to stop", not "the accept failed". Reporting them as errors // would make every deliberate shutdown look like a fault to the Java side. - if(acceptErrno == EBADF || acceptErrno == EINVAL || acceptErrno == EINTR) { + if(acceptErrno == EBADF || acceptErrno == EINVAL) { errorMessage = nil; } else { errorMessage = @"Accept failed"; From a51f97af01a61606aa207c677399cf26d21ba8b6 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 27 Jul 2026 16:50:50 +0300 Subject: [PATCH 18/24] Let a caller preflight the build gate, not just platform capability isSocketSupported() answers whether the platform CAN bind a loopback server socket. startSocketServer can still refuse for an unrelated reason -- a release build -- so anyone using the first as a preflight for the second would be misled by an answer that was true and irrelevant. Rather than folding the gate into isSocketSupported, which would conflate a capability with a policy, the policy is now queryable on its own: isSocketServerAllowedOnThisBuild(). startSocketServer checks both and refuses on whichever says no first, and the capability javadoc says plainly that it ignores the gate and points at the other half. Also accept TIMED_WAITING alongside WAITING when waiting for the reader thread to park. The wait is sound as it stands -- once parked the reader STAYS parked until close() notifies it, so polling cannot miss the window, which is what separates this from sleeping and hoping -- but a future timed wait inside the transport would have turned it into a ten second stall instead of an immediate pass. Declined, with reasoning in the PR: StandardCharsets in place of the "UTF-8" literal. Core has 0 files using StandardCharsets and 55 using the literal, so this would be the first such usage and inconsistent with the established convention; ParparVM's StandardCharsets.UTF_8 is itself Charset.forName("UTF-8"), so there is no lookup saved either. A sweep of all 55 would be a reasonable separate change if the convention should move. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/mcp/MCP.java | 18 ++++++++++++++- .../mcp/MCPLoopbackSocketTransportTest.java | 10 +++++++-- .../mcp/MCPReleaseBuildGateTest.java | 22 +++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/CodenameOne/src/com/codename1/mcp/MCP.java b/CodenameOne/src/com/codename1/mcp/MCP.java index 7f3265e42c2..d228700e969 100644 --- a/CodenameOne/src/com/codename1/mcp/MCP.java +++ b/CodenameOne/src/com/codename1/mcp/MCP.java @@ -98,6 +98,11 @@ public static boolean isStdioSupported() { /// Whether a loopback socket transport is available on this platform - either one the /// port registered itself, or the portable one, which needs /// [com.codename1.io.Socket#isLoopbackServerSocketSupported()]. + /// + /// This is a question about CAPABILITY and deliberately ignores the release build + /// gate, so it is not a complete preflight for [#startSocketServer(int)] on its own - + /// a platform that can bind may still refuse to serve. Ask + /// [#isSocketServerAllowedOnThisBuild()] for the other half. public static boolean isSocketSupported() { return socketTransportFactory != null || com.codename1.io.Socket.isLoopbackServerSocketSupported(); @@ -189,9 +194,20 @@ public static synchronized boolean isAllowedOnReleaseBuilds() { return allowOnReleaseBuilds; } + /// Whether the socket server is allowed to run on THIS build, which is the other half + /// of the question [#isSocketSupported()] answers: that one asks whether the platform + /// can bind a loopback server socket, this one whether it is permitted to serve. + /// [#startSocketServer(int)] needs both, and refuses on the first that says no. + /// + /// True on a development build, or on any build where + /// [#setAllowOnReleaseBuilds(boolean)] has been used. + public static synchronized boolean isSocketServerAllowedOnThisBuild() { + return allowOnReleaseBuilds || Display.getInstance().isDebuggableBuild(); + } + /// Throws unless this build may serve MCP over a socket. private static void checkAllowedOnThisBuild() { - if (allowOnReleaseBuilds || Display.getInstance().isDebuggableBuild()) { + if (isSocketServerAllowedOnThisBuild()) { return; } throw new IllegalStateException( diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java index 446b05cb2ee..cd077e1ef70 100644 --- a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java @@ -329,8 +329,14 @@ void closeUnblocksAPendingRead() throws Exception { // Park the reader before closing, so this exercises "close wakes a waiting reader" // rather than "close had already been called". WAITING is precisely the state // lock.wait() puts it in. - await("the reader to park waiting for a client", - () -> reader.getState() == Thread.State.WAITING); + // Polling for a parked state is not the same as sleeping and hoping: once the + // reader reaches wait() it STAYS there until close() notifies it, so this cannot + // miss the window. TIMED_WAITING is accepted too, so a future timed wait in the + // transport would not silently turn this into a ten second stall. + await("the reader to park waiting for a client", () -> { + Thread.State state = reader.getState(); + return state == Thread.State.WAITING || state == Thread.State.TIMED_WAITING; + }); t.close(); reader.join(3000); diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java index bf8fcef6830..c6fd44e7b48 100644 --- a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java @@ -99,6 +99,28 @@ void theOverrideIsOffByDefault() { assertFalse(MCP.isAllowedOnReleaseBuilds()); } + @Test + void theBuildGateIsQueryableSeparatelyFromPlatformCapability() { + // A caller preflighting with isSocketSupported() alone would be misled: that asks + // whether the platform CAN bind, and a build can be capable yet not permitted. + implementation.setServerSocketAvailable(true); + + implementation.setDebuggableBuild(false); + assertFalse(MCP.isSocketServerAllowedOnThisBuild(), + "a release build must report that it is not allowed to serve"); + assertTrue(MCP.isSocketSupported(), + "capability is unchanged by the gate; the platform can still bind"); + + implementation.setDebuggableBuild(true); + assertTrue(MCP.isSocketServerAllowedOnThisBuild(), + "a development build must report that it is allowed to serve"); + + implementation.setDebuggableBuild(false); + MCP.setAllowOnReleaseBuilds(true); + assertTrue(MCP.isSocketServerAllowedOnThisBuild(), + "the override must be reflected in the preflight, not only in the start"); + } + @Test void aBlockedStartLeavesNoServerRunning() { // The gate runs before the transport is registered or opened, so a refusal must From f8d3ec8b0fe81da0989ed01dc08dfc15d258bb4c Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 27 Jul 2026 20:08:59 +0300 Subject: [PATCH 19/24] Close the client's output stream too, and fix the guide anchor close() forgot the output stream rather than closing it. A writer that had already captured it would go on writing into a session that had ended, and the underlying socket stayed open until the connection callback happened to unwind. Both streams now come out of their fields under the lock and are closed outside it, through the same closeQuietly helpers attach() uses. New test covers it, control-tested against the previous form. The guide anchor was [[Development builds only]] -- an anchor ID cannot contain spaces, so the cross-reference to it was malformed even though asciidoctor did not fail the build. Now [[development-builds-only]], referenced with explicit link text so the rendered wording is unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- .../mcp/MCPLoopbackSocketTransport.java | 19 ++++++------ .../developer-guide/MCP-Headless-API.asciidoc | 4 +-- .../mcp/MCPLoopbackSocketTransportTest.java | 31 +++++++++++++++++++ 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java index 2186b0fd91f..c5206a342d4 100644 --- a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java +++ b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java @@ -318,14 +318,17 @@ private boolean isClosed() { @Override public void close() { Socket.StopListening l; - // Taken out of the field under the lock precisely so it can be closed below, - // outside the lock: closing is what unblocks a reader parked in read(). + // Both streams come out of their fields under the lock so they can be closed + // below, outside it: closing is what unblocks a reader parked in read(), and it + // takes the lock on its way out. InputStream is; // NOPMD closed below, deliberately outside the lock + OutputStream os; // NOPMD closed below, outside the lock synchronized (lock) { closed = true; l = listening; listening = null; is = in; + os = out; in = null; out = null; lock.notifyAll(); @@ -335,13 +338,11 @@ public void close() { if (l != null) { l.stop(); } - if (is != null) { - try { - is.close(); - } catch (IOException ignored) { - // best effort: closing is what unblocks a pending read - } - } + // Closing the output as well as the input: forgetting the field is not enough, + // because a writer that already captured it would go on writing to a session that + // has ended, and the socket would stay open until the connection callback unwound. + closeQuietly(is); + closeQuietly(os); } /// The per-connection callback the socket API instantiates. Public with a no-argument diff --git a/docs/developer-guide/MCP-Headless-API.asciidoc b/docs/developer-guide/MCP-Headless-API.asciidoc index 79acd9ac230..fb4e6a1e0b7 100644 --- a/docs/developer-guide/MCP-Headless-API.asciidoc +++ b/docs/developer-guide/MCP-Headless-API.asciidoc @@ -4,7 +4,7 @@ The Model Context Protocol (MCP) headless API lets an application expose itself The API reuses the accessibility semantics tree. The same immutable tree that describes the screen to VoiceOver and TalkBack also describes it to the agent, so any application is drivable without extra code. Application specific data and actions are exposed through the existing `com.codename1.ai.Tool` contract. -The socket transport runs anywhere the port can bind the loopback interface, which includes an application running on a device, so an agent can attach to a build on a phone as well as to the simulator. It's blocked on a release build; see <>. The stdio transport needs process standard input and so is supported by the JavaSE port, which powers the simulator and the desktop tooling. +The socket transport runs anywhere the port can bind the loopback interface, which includes an application running on a device, so an agent can attach to a build on a phone as well as to the simulator. It's blocked on a release build; see <>. The stdio transport needs process standard input and so is supported by the JavaSE port, which powers the simulator and the desktop tooling. == The MCP menu @@ -34,7 +34,7 @@ include::../demos/common/src/main/java/com/codenameone/developerguide/snippets/M The stdio transport is the standard MCP local transport, exchanging newline delimited JSON-RPC messages. While it runs, application logging is redirected away from standard output so it can't corrupt the protocol stream. The stdio transport lives in the JavaSE port because it needs process standard input, which isn't available on every target. -[[Development builds only]] +[[development-builds-only]] == Development builds only The socket server refuses to bind on a release build and throws an `IllegalStateException` instead. diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java index cd077e1ef70..187e7460d9b 100644 --- a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java @@ -344,6 +344,37 @@ void closeUnblocksAPendingRead() throws Exception { assertNull(out[0], "end of transport is signalled by a null message"); } + @Test + void closingTheTransportClosesBothClientStreams() throws Exception { + // Forgetting the fields is not enough. A writer that already captured the output + // stream would go on writing into a session that has ended, and the socket would + // stay open until the connection callback happened to unwind. + final boolean[] inClosed = new boolean[1]; + final boolean[] outClosed = new boolean[1]; + transport = new MCPLoopbackSocketTransport(47811); + transport.attach(new InputStream() { + @Override + public int read() { + return -1; + } + + @Override + public void close() { + inClosed[0] = true; + } + }, new ByteArrayOutputStream() { + @Override + public void close() { + outClosed[0] = true; + } + }); + + transport.close(); + transport = null; + assertTrue(inClosed[0], "close must close the client's input stream"); + assertTrue(outClosed[0], "close must close the client's output stream too"); + } + @Test void aPipedClientRoundTrips() throws Exception { // Closest thing to a real socket without one: the client end is a live pipe, so From ad7e0379cd17139792efb06b38323d61eb6f7d8d Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 28 Jul 2026 03:22:49 +0300 Subject: [PATCH 20/24] Read the provisioning profile once, and decode frames without a copy isDebuggableBuild() re-read and re-parsed embedded.mobileprovision on every call. The profile is fixed for the life of the process, and this is consulted by the MCP gate and exposed publicly through Display.isDebuggableBuild(), so a caller is free to ask repeatedly -- file IO plus a plist parse each time is a poor answer to that. It is now computed once under dispatch_once. Verified by re-running the extracted-function harness: the four fixtures still classify identically (development, distribution-with-beta-reports-active, corrupt, missing), and a second harness with the once-token left intact shows the second call returning the first answer without re-reading, which is the cache doing its job. toUtf8 used buffer.toByteArray(), which copies the whole payload before decoding it -- megabytes of pointless duplication for a frame near the ceiling. ByteArrayOutputStream.toString(String) decodes straight out of the internal buffer, and ParparVM implements it as new String(buf, 0, count, charsetName), so the saving is real on device and not only on the JVM. Co-Authored-By: Claude Opus 5 (1M context) --- .../com/codename1/mcp/MCPLoopbackSocketTransport.java | 4 +++- Ports/iOSPort/nativeSources/IOSNative.m | 11 ++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java index c5206a342d4..61cdbee4eed 100644 --- a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java +++ b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java @@ -289,8 +289,10 @@ private static boolean append(ByteArrayOutputStream buffer, int b) { return true; } + /// Decodes straight out of the buffer. toByteArray() would copy the whole payload + /// first, which for a frame near the ceiling is megabytes of pointless duplication. private static String toUtf8(ByteArrayOutputStream buffer) throws IOException { - return new String(buffer.toByteArray(), "UTF-8"); + return buffer.toString("UTF-8"); } @Override diff --git a/Ports/iOSPort/nativeSources/IOSNative.m b/Ports/iOSPort/nativeSources/IOSNative.m index 21449125599..ef60c5babc3 100644 --- a/Ports/iOSPort/nativeSources/IOSNative.m +++ b/Ports/iOSPort/nativeSources/IOSNative.m @@ -10871,6 +10871,13 @@ JAVA_BOOLEAN com_codename1_impl_ios_IOSNative_isDebuggableBuild___R_boolean(CN1_ // no provisioning profile to inspect, which is what the device path relies on. return JAVA_TRUE; #else + // The provisioning profile is fixed for the life of the process, so read and parse it + // once. This is consulted by the MCP gate and is public through + // Display.isDebuggableBuild(), so a caller is free to ask repeatedly, and file IO plus + // a plist parse per call would be a poor answer to that. + static JAVA_BOOLEAN cachedDebuggable = JAVA_FALSE; + static dispatch_once_t debuggableOnce; + dispatch_once(&debuggableOnce, ^{ POOL_BEGIN(); JAVA_BOOLEAN result = JAVA_FALSE; // get-task-allow is the entitlement that lets a debugger attach. Development and @@ -10921,7 +10928,9 @@ JAVA_BOOLEAN com_codename1_impl_ios_IOSNative_isDebuggableBuild___R_boolean(CN1_ // Every path that could not read an answer leaves result JAVA_FALSE: an unreadable or // unexpected profile means "treat this as a release build", which withholds the // facility rather than exposing it. - return result; + cachedDebuggable = result; + }); + return cachedDebuggable; #endif } From 8ad47e88f7505cb1cda24dc0b185f8399267c34f Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 28 Jul 2026 04:23:09 +0300 Subject: [PATCH 21/24] Revert the frame decode: core compiles against CLDC11, not the JDK buffer.toString("UTF-8") broke the build. Core is compiled with -bootclasspath Ports/CLDC11/dist/CLDC11.jar, and CLDC11's ByteArrayOutputStream declares only the no-argument toString(). The overload exists on the JDK and in the ParparVM runtime, which is why a Maven build of core accepts it and the Ant build then fails -- and why checking the ParparVM sources, as I did, was not a check of anything core actually compiles against. Restored the toByteArray() form with a note on the method so the next person is not tempted by the same avoidable copy. Also declare string.h explicitly for memset. It resolves today only because Foundation drags it in, which is not something to rely on. Verified this time by running the compile CI runs -- ant -buildfile CodenameOne/build.xml jar against a locally built CLDC11 jar -- rather than inferring from Maven, which cannot catch this class of error at all. Co-Authored-By: Claude Opus 5 (1M context) --- .../com/codename1/mcp/MCPLoopbackSocketTransport.java | 9 ++++++--- Ports/iOSPort/nativeSources/SocketImpl.m | 1 + 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java index 61cdbee4eed..e41f8a6d4f8 100644 --- a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java +++ b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java @@ -289,10 +289,13 @@ private static boolean append(ByteArrayOutputStream buffer, int b) { return true; } - /// Decodes straight out of the buffer. toByteArray() would copy the whole payload - /// first, which for a frame near the ceiling is megabytes of pointless duplication. + /// Note for anyone tempted by ByteArrayOutputStream.toString(String), which would + /// decode without toByteArray()'s copy: core is compiled against CLDC11 + /// (Ports/CLDC11), whose ByteArrayOutputStream declares only the no-argument + /// toString(). The overload exists on the JDK and in the ParparVM runtime, so a Maven + /// build of core compiles it happily and the Ant build then fails. private static String toUtf8(ByteArrayOutputStream buffer) throws IOException { - return buffer.toString("UTF-8"); + return new String(buffer.toByteArray(), "UTF-8"); } @Override diff --git a/Ports/iOSPort/nativeSources/SocketImpl.m b/Ports/iOSPort/nativeSources/SocketImpl.m index 97fc8f41cfd..dbaed59d326 100644 --- a/Ports/iOSPort/nativeSources/SocketImpl.m +++ b/Ports/iOSPort/nativeSources/SocketImpl.m @@ -27,6 +27,7 @@ #include #include #include +#include #include #include "xmlvm.h" #import "CodenameOne_GLViewController.h" From 6734cb2f85eb7592d8300275a4013caac1056eb6 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 28 Jul 2026 06:08:31 +0300 Subject: [PATCH 22/24] Do not announce a fault when a listener is deliberately stopped Closing the listening socket is how a stop is delivered, and it surfaces in the loop as a failed accept. The null-connection branch then reported that through connectionError, announcing a fault that never happened, and passed a null connection to getSocketErrorCode/Message on the way. It now breaks out when a failed accept coincides with a stop, before any callback object is built. The backoff after a genuine accept failure is slept in slices with the flag re-tested between them, so a stop arriving during the pause is acted on within 50ms rather than after the full 500. Tests: assert the exception message is non-null before matching on it, in the two places that were still calling getMessage() straight into contains() -- the same guard the refusal helper already had. And the wait helper's deadline is System.nanoTime() rather than currentTimeMillis, so an NTP step mid-wait cannot cut the wait short or stretch it. Verified with the Ant CLDC11 core compile FIRST this time, then the Maven build, 41 MCP tests and PMD. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/io/Socket.java | 24 +++++++++++++++---- .../mcp/MCPLoopbackSocketTransportTest.java | 7 ++++-- .../mcp/MCPLoopbackTransportOpenTest.java | 7 ++++-- .../mcp/MCPReleaseBuildGateTest.java | 2 ++ 4 files changed, 32 insertions(+), 8 deletions(-) diff --git a/CodenameOne/src/com/codename1/io/Socket.java b/CodenameOne/src/com/codename1/io/Socket.java index fdf10500be7..ef544da9836 100644 --- a/CodenameOne/src/com/codename1/io/Socket.java +++ b/CodenameOne/src/com/codename1/io/Socket.java @@ -230,6 +230,10 @@ class Listener implements StopListening, Runnable { /// listener would quietly resurrect itself instead of stopping. private final AtomicBoolean stopped = new AtomicBoolean(); + /// The backoff after a failed accept, split so a stop is noticed promptly. + private static final int BACKOFF_SLICES = 10; + private static final int BACKOFF_SLICE_MS = 50; + @Override public void run() { try { @@ -246,6 +250,12 @@ public void run() { } break; } + if (connection == null && stopped.get()) { + // Closing the listening socket is how a stop is delivered, and + // it surfaces here as a failed accept. Reporting that through + // connectionError would announce a fault that never happened. + break; + } final SocketConnection sc = (SocketConnection) scClass.newInstance(); if (connection != null) { sc.setConnected(true); @@ -265,10 +275,16 @@ public void run() { // full speed, burning a core and filling the log. Pause so a // persistent failure is slow and visible instead of hot. Only // the failure path waits; a served connection never gets here. - try { - Thread.sleep(500); - } catch (InterruptedException interrupted) { - // fall through: the loop re-tests stopped straight away + // + // Slept in slices, re-testing the flag between them, so a stop + // arriving during the pause is acted on within a slice rather + // than after the whole backoff. + for (int slice = 0; slice < BACKOFF_SLICES && !stopped.get(); slice++) { + try { + Thread.sleep(BACKOFF_SLICE_MS); + } catch (InterruptedException interrupted) { + break; // the loop re-tests stopped straight away + } } } } diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java index 187e7460d9b..e9490697754 100644 --- a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java @@ -65,8 +65,11 @@ void closeTransport() { /// on has got anywhere - which is how timing-based tests turn flaky. private static void await(String what, java.util.concurrent.Callable condition) throws Exception { - long deadline = System.currentTimeMillis() + 10000; - while (System.currentTimeMillis() < deadline) { + // nanoTime, not currentTimeMillis: a wall clock can be stepped by NTP mid-wait, + // which would either cut the wait short or extend it, and a test that fails when + // the clock is adjusted is worse than one that sleeps. + long deadline = System.nanoTime() + 10000L * 1000000L; + while (System.nanoTime() < deadline) { if (condition.call()) { return; } diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackTransportOpenTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackTransportOpenTest.java index 57fe6ff99de..c9985e747b4 100644 --- a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackTransportOpenTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackTransportOpenTest.java @@ -29,6 +29,7 @@ import java.io.IOException; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -66,8 +67,10 @@ void aFailedListenNeitherStrandsTheRegistrationNorEscapesAsRuntime() throws Exce // open(). A RuntimeException would kill the thread before it cleared its running // flag, leaving the server permanently "running" with nothing behind it. IOException failure = assertThrows(IOException.class, () -> first.open()); - assertTrue(failure.getMessage().contains("Failed to listen"), - "the failure should name what went wrong, got: " + failure.getMessage()); + String message = failure.getMessage(); + assertNotNull(message, "the failure must explain itself"); + assertTrue(message.contains("Failed to listen"), + "the failure should name what went wrong, got: " + message); // And the registration must be free, or every later transport would be refused on // the grounds that an agent is already being served. diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java index c6fd44e7b48..cd7825d0284 100644 --- a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPReleaseBuildGateTest.java @@ -138,6 +138,7 @@ void theGateIsCheckedBeforeTransportAvailability() { implementation.setServerSocketAvailable(false); String message = assertThrows(IllegalStateException.class, () -> MCP.startSocketServer(47898)).getMessage(); + assertNotNull(message, "the refusal must explain itself"); assertTrue(message.contains(GATE), "the build gate must be evaluated first, got: " + message); assertFalse(MCP.isRunning()); @@ -152,6 +153,7 @@ void aPlatformThatCannotBindLoopbackFailsOnTheCallingThread() { implementation.setServerSocketAvailable(false); String message = assertThrows(IllegalStateException.class, () -> MCP.startSocketServer(47897)).getMessage(); + assertNotNull(message, "the refusal must explain itself"); assertTrue(message.contains("No socket MCP transport"), "an unbindable platform must say so synchronously, got: " + message); assertFalse(MCP.isRunning(), "nothing must be left running after that refusal"); From 870b62e8fae019eb8494cc2cf7252c4a46510a97 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 28 Jul 2026 07:00:12 +0300 Subject: [PATCH 23/24] Read frames in blocks, and close the visibility gap on the transport factories readLine consumed the stream a byte at a time, so a frame near the 8MB ceiling cost millions of single-byte reads -- cheap to trigger from the other side of a loopback connection. It now reads in 8KB blocks. Buffering is only safe if bytes past the delimiter are KEPT rather than swallowed, which was the reason the byte-at-a-time loop existed. They are held in a per-transport chunk and consumed by the next call, and the chunk is discarded whenever the attached stream is not the one that filled it -- otherwise a new client's first message would be a dead client's tail. A test covers both halves, and control-testing the second one returns {"id":3} to the new client instead of its own {"id":9}, which is one session's data delivered to another. hasPlatformSocketTransport, isStdioSupported and isSocketSupported read the factory fields without holding the class monitor while everything else in the class takes it. The reads and the two setters are now synchronized. I tried volatile first; PMD forbids it here (AvoidUsingVolatile), which also matches the review preference expressed earlier on this PR. getStoppedListeners returns an unmodifiable snapshot rather than the live list, so a test cannot mutate what a later assertion reads. Verified with the Ant CLDC11 core compile, the Maven build, 42 MCP tests and PMD. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/mcp/MCP.java | 14 +++-- .../mcp/MCPLoopbackSocketTransport.java | 62 ++++++++++++++----- .../mcp/MCPLoopbackSocketTransportTest.java | 16 +++++ .../TestCodenameOneImplementation.java | 8 ++- 4 files changed, 78 insertions(+), 22 deletions(-) diff --git a/CodenameOne/src/com/codename1/mcp/MCP.java b/CodenameOne/src/com/codename1/mcp/MCP.java index d228700e969..45dbc39d266 100644 --- a/CodenameOne/src/com/codename1/mcp/MCP.java +++ b/CodenameOne/src/com/codename1/mcp/MCP.java @@ -48,6 +48,10 @@ /// ``` public final class MCP { private static MCPServer server; + /// Every access to these goes through the class monitor, because the port that + /// registers a transport and the code asking whether one exists are not necessarily + /// the same thread. The accessors below are synchronized for that reason rather than + /// the fields being volatile, which this codebase does not use. private static StdioTransportFactory stdioTransportFactory; private static SocketTransportFactory socketTransportFactory; /// Lifts the release build block; see [#setAllowOnReleaseBuilds(boolean)]. @@ -74,24 +78,24 @@ public interface SocketTransportFactory { } /// Registers the platform stdio transport factory. Called by the JavaSE port. - public static void setStdioTransportFactory(StdioTransportFactory factory) { + public static synchronized void setStdioTransportFactory(StdioTransportFactory factory) { stdioTransportFactory = factory; } /// Registers the platform socket transport factory. Called by the JavaSE port. - public static void setSocketTransportFactory(SocketTransportFactory factory) { + public static synchronized void setSocketTransportFactory(SocketTransportFactory factory) { socketTransportFactory = factory; } /// Whether the PORT supplied a transport of its own. Distinct from /// [#isSocketSupported()], which also counts the portable fallback - the fallback /// must not see itself as already installed. - static boolean hasPlatformSocketTransport() { + static synchronized boolean hasPlatformSocketTransport() { return socketTransportFactory != null; } /// Whether an stdio transport is available on this platform. - public static boolean isStdioSupported() { + public static synchronized boolean isStdioSupported() { return stdioTransportFactory != null; } @@ -103,7 +107,7 @@ public static boolean isStdioSupported() { /// gate, so it is not a complete preflight for [#startSocketServer(int)] on its own - /// a platform that can bind may still refuse to serve. Ask /// [#isSocketServerAllowedOnThisBuild()] for the other half. - public static boolean isSocketSupported() { + public static synchronized boolean isSocketSupported() { return socketTransportFactory != null || com.codename1.io.Socket.isLoopbackServerSocketSupported(); } diff --git a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java index e41f8a6d4f8..84c8d774579 100644 --- a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java +++ b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java @@ -54,6 +54,18 @@ public final class MCPLoopbackSocketTransport implements MCPTransport { /// behaved or hostile client can make the server allocate. static final int MAX_FRAME_BYTES = 8 * 1024 * 1024; + /// Bulk reads land here and are consumed a byte at a time. Whatever follows a + /// delimiter stays put for the next message, which is what makes buffering safe: the + /// bytes are held rather than swallowed. + /// + /// Touched only by the reader thread, and reset in readMessage whenever the attached + /// stream is not the one that filled it, so a new client never inherits a dead + /// client's tail. + private final byte[] chunk = new byte[8192]; + private int chunkPos; + private int chunkLen; + private InputStream chunkSource; // NOPMD borrowed identity, never read from or closed + private final int port; private final Object lock = new Object(); private Socket.StopListening listening; @@ -212,6 +224,14 @@ public String readMessage() throws IOException { } stream = in; } + if (stream != chunkSource) { // NOPMD identity: is this the stream we buffered? + // A different client. Anything left from the previous one belongs to a + // session that has ended and must not be read as this client's first + // message. + chunkSource = stream; + chunkPos = 0; + chunkLen = 0; + } String line = readLine(stream); if (line != null) { return line; @@ -227,9 +247,10 @@ public String readMessage() throws IOException { /// Reads one newline-delimited UTF-8 message. Returns null at end of stream, which /// includes a frame cut short by the client going away. /// - /// Read byte at a time rather than through a buffered reader: a buffer would swallow - /// bytes belonging to the next message, and this transport hands the same stream back - /// after a reconnect. + /// Reads in blocks rather than a byte at a time - a frame near the ceiling would + /// otherwise cost millions of single-byte reads. Buffering is safe here because + /// whatever follows the delimiter is KEPT in the chunk for the next call, so no bytes + /// belonging to the next message are swallowed. private String readLine(InputStream stream) throws IOException { ByteArrayOutputStream buffer = new ByteArrayOutputStream(); // A carriage return is only part of the delimiter when a newline follows it, so it @@ -237,20 +258,29 @@ private String readLine(InputStream stream) throws IOException { // a payload that happened to contain one. boolean pendingCarriageReturn = false; while (true) { - int b; - try { - b = stream.read(); - } catch (IOException ex) { - return null; // a read error on the client is a disconnect - } - if (b < 0) { - // End of stream with no delimiter: the frame is truncated, so this is a - // disconnect and not a message, however many bytes arrived first. Handing - // the partial frame up would have the server fail to parse it, then fail - // to answer it (the peer is already gone), and end the loop - when the - // whole point of this transport is to stay bound and await a reconnect. - return null; + if (chunkPos >= chunkLen) { + int read; + try { + read = stream.read(chunk, 0, chunk.length); + } catch (IOException ex) { + return null; // a read error on the client is a disconnect + } + if (read < 0) { + // End of stream with no delimiter: the frame is truncated, so this is + // a disconnect and not a message, however many bytes arrived first. + // Handing the partial frame up would have the server fail to parse it, + // then fail to answer it (the peer is already gone), and end the loop - + // when the whole point of this transport is to stay bound and await a + // reconnect. + return null; + } + chunkPos = 0; + chunkLen = read; + if (read == 0) { + continue; // nothing this time; ask again rather than call it EOF + } } + int b = chunk[chunkPos++] & 0xff; if (pendingCarriageReturn) { pendingCarriageReturn = false; if (b == '\n') { diff --git a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java index e9490697754..9726b9a35e1 100644 --- a/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java @@ -102,6 +102,22 @@ void splitsConsecutiveMessagesArrivingTogether() throws Exception { assertEquals("{\"id\":2}", t.readMessage()); } + @Test + void bufferedReadsDoNotSwallowTheNextClientsBytes() throws Exception { + // Reading in blocks means a read can pull in bytes past the delimiter. Those + // belong to the NEXT message on the same connection and must be kept, but they + // must not survive into a different client's session. + MCPLoopbackSocketTransport t = attached("{\"id\":1}\n{\"id\":2}\n{\"id\":3}\n"); + assertEquals("{\"id\":1}", t.readMessage()); + assertEquals("{\"id\":2}", t.readMessage(), "carried-over bytes must be readable"); + + // A new client arrives while the previous stream still had "{id:3}" buffered. + t.attach(new ByteArrayInputStream("{\"id\":9}\n".getBytes("UTF-8")), + new ByteArrayOutputStream()); + assertEquals("{\"id\":9}", t.readMessage(), + "the new client's first message must not be the old client's leftovers"); + } + @Test void toleratesCarriageReturnsBeforeTheDelimiter() throws Exception { MCPLoopbackSocketTransport t = attached("{\"id\":1}\r\n"); diff --git a/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java b/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java index f3e98a602cd..187484c0fa0 100644 --- a/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java +++ b/maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java @@ -2803,8 +2803,14 @@ public void stopListeningSocket(int port, boolean loopbackOnly) { stoppedListeners.add(port + ":" + loopbackOnly); } + /// A snapshot, not the live list. Handing back the real one lets a test mutate what a + /// later assertion reads, and iterating a synchronized list safely needs external + /// locking that a test would have to remember. public java.util.List getStoppedListeners() { - return stoppedListeners; + synchronized (stoppedListeners) { + return java.util.Collections.unmodifiableList( + new java.util.ArrayList(stoppedListeners)); + } } private boolean loopbackSupportedOnlyOnFirstQuery; From 9a21ba940c8f68901127c80815fc4ef6b006f622 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 28 Jul 2026 10:13:35 +0300 Subject: [PATCH 24/24] Report a listener that cannot bind, and back off exponentially when it retries connectionError was an empty method, so a listener that could never bind failed silently: startSocketServer returns, the server reports itself running, and nothing can attach - a port already in use being the ordinary way that happens. It now logs, de-duplicated on the message so a permanent failure says so once rather than once per retry, and falls back to stderr because the log routes through the platform implementation, which may not be registered when a server starts early in initialization. The retry itself was a fixed 500ms, so a port that can never be bound meant two callbacks a second for the life of the process. The backoff now starts at 50ms, doubles while the failure persists and is capped at 5s, and resets as soon as an accept succeeds so a listener that saw one bad moment is not left sluggish. Still slept in slices, so stopping is noticed within 50ms regardless of how long the backoff has grown. Also dropped "which this codebase does not use" from a comment. It was a global claim in a local rationale, and the sort of thing that quietly becomes false. Verified with the Ant CLDC11 core compile first, then Maven, 42 MCP tests, PMD. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/io/Socket.java | 21 ++++++++++++++---- CodenameOne/src/com/codename1/mcp/MCP.java | 5 +++-- .../mcp/MCPLoopbackSocketTransport.java | 22 ++++++++++++++++++- 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/CodenameOne/src/com/codename1/io/Socket.java b/CodenameOne/src/com/codename1/io/Socket.java index ef544da9836..12fcbfd1fb6 100644 --- a/CodenameOne/src/com/codename1/io/Socket.java +++ b/CodenameOne/src/com/codename1/io/Socket.java @@ -230,9 +230,15 @@ class Listener implements StopListening, Runnable { /// listener would quietly resurrect itself instead of stopping. private final AtomicBoolean stopped = new AtomicBoolean(); - /// The backoff after a failed accept, split so a stop is noticed promptly. - private static final int BACKOFF_SLICES = 10; + /// The backoff after a failed accept. It starts short, doubles while the + /// failure persists and is capped, so a port that can never be bound settles + /// into one attempt every few seconds instead of two a second forever. Reset + /// as soon as an accept succeeds, so a listener that saw one bad moment is not + /// left sluggish. Slept in slices so a stop is still noticed promptly. + private static final int BACKOFF_MIN_MS = 50; + private static final int BACKOFF_MAX_MS = 5000; private static final int BACKOFF_SLICE_MS = 50; + private int backoffMs = BACKOFF_MIN_MS; @Override public void run() { @@ -258,6 +264,7 @@ public void run() { } final SocketConnection sc = (SocketConnection) scClass.newInstance(); if (connection != null) { + backoffMs = BACKOFF_MIN_MS; // healthy again sc.setConnected(true); Display.getInstance().startThread(new Runnable() { @Override @@ -279,12 +286,18 @@ public void run() { // Slept in slices, re-testing the flag between them, so a stop // arriving during the pause is acted on within a slice rather // than after the whole backoff. - for (int slice = 0; slice < BACKOFF_SLICES && !stopped.get(); slice++) { + int waited = 0; + while (waited < backoffMs && !stopped.get()) { + int slice = Math.min(BACKOFF_SLICE_MS, backoffMs - waited); try { - Thread.sleep(BACKOFF_SLICE_MS); + Thread.sleep(slice); } catch (InterruptedException interrupted) { break; // the loop re-tests stopped straight away } + waited += slice; + } + if (backoffMs < BACKOFF_MAX_MS) { + backoffMs = Math.min(backoffMs * 2, BACKOFF_MAX_MS); } } } diff --git a/CodenameOne/src/com/codename1/mcp/MCP.java b/CodenameOne/src/com/codename1/mcp/MCP.java index 45dbc39d266..7a03a1bcd02 100644 --- a/CodenameOne/src/com/codename1/mcp/MCP.java +++ b/CodenameOne/src/com/codename1/mcp/MCP.java @@ -50,8 +50,9 @@ public final class MCP { private static MCPServer server; /// Every access to these goes through the class monitor, because the port that /// registers a transport and the code asking whether one exists are not necessarily - /// the same thread. The accessors below are synchronized for that reason rather than - /// the fields being volatile, which this codebase does not use. + /// the same thread. The accessors below are synchronized for that reason, which also + /// keeps every read and write of this state on the one lock the rest of the class + /// already takes. private static StdioTransportFactory stdioTransportFactory; private static SocketTransportFactory socketTransportFactory; /// Lifts the release build block; see [#setAllowOnReleaseBuilds(boolean)]. diff --git a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java index 84c8d774579..5e715ae6fea 100644 --- a/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java +++ b/CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java @@ -22,6 +22,7 @@ */ package com.codename1.mcp; +import com.codename1.io.Log; import com.codename1.io.Socket; import com.codename1.io.SocketConnection; @@ -383,9 +384,28 @@ public void close() { /// The per-connection callback the socket API instantiates. Public with a no-argument /// constructor because [Socket#listenLoopback] creates it reflectively. public static final class Connection extends SocketConnection { + /// The last failure reported, so a listener that cannot bind at all says so once + /// instead of once per retry. Only ever touched from the listener thread. + private static String lastReportedError; + @Override public void connectionError(int errorCode, String message) { - // The listen loop reports its own failures; nothing useful to add per client. + // Without this the failure is silent: startSocketServer returns, the server + // reports itself running, and nothing can ever attach - a port already in use + // being the ordinary way that happens. Logged rather than thrown because this + // arrives on the listener thread, long after the caller has gone. + String description = "MCP socket listener failed: " + message + " (" + errorCode + ")"; + if (description.equals(lastReportedError)) { + return; // the same failure retrying; saying so once is enough + } + lastReportedError = description; + try { + Log.p(description, Log.ERROR); + } catch (Throwable loggingFailed) { + // The log routes through the platform implementation, which may not be + // registered yet when a server is started early in initialization. + System.err.println("[cn1.mcp] " + description); + } } @Override