feat: webview support for real iOS devices#246
Merged
Conversation
Adds WebView list/goto/reload/back/forward/content/evaluate/waitForLoadState for physical iOS devices. A real device can't dlopen an unsigned dylib like the simulator path, so the agent is injected into the foreground app via LLDB over the go-ios CoreDevice debug proxy, then driven over a forwarded HTTP/JSON-RPC port using the shared agent protocol (agentRequest/WebViewInfo). All device-specific code lives in devices/ios_device_webview.go so it never conflicts with the shared simulator/Android webview code. Vendors two small, dependency-light packages from go-ios that aren't in the released v1.0.211 this module uses: - devices/ios/debuggertools: ObjC-runtime-over-GDB calls, used to detect the foreground app without WDA. - devices/ios/debugserver: GDBServer with the bufio.Scanner buffer raised to 64MB so the multi-hundred-KB jGetLoadedDynamicLibrariesInfos image-list packet can be read; the 64KB default made LLDB load zero images (no symbols -> expr injection failed).
The injected agent persists inside the app after LLDB detaches, but the in-memory port cache is lost between CLI invocations, so every command re-ran the full (~13s) LLDB injection. Before injecting, probe the device agent port range (27042-27051): forward a local port to each and check whether the HTTP/JSON-RPC agent already answers. If one does, reuse it and skip findForegroundApp + LLDB entirely. A refused device port fails fast, so the cold-path tax is negligible. Caveat: this reuses whatever agent is alive on the device in that range; if the foreground app changed but the previous app is still running, it may talk to the previous app's agent.
…agent LLDB compiles the injected agent expression in ARC. Holding the result of evaluateJavaScript (jsResult = r) added a retain that, for any heap (non-tagged) value, got over-released when WebKit drained its delivery autorelease pool — crashing the target app right after the value was returned (EXC_BREAKPOINT in _CFRelease during objc_autoreleasePoolPop). Small/tagged results (short strings, numbers) survived, which is why `list` and trivial evals worked but `content` (large outerHTML) reliably killed the app. Rework the evaluate path to never hold WebKit's result object: the injected JS now returns JSON.stringify([ok, value]); the completion handler copies those UTF-8 bytes into C memory (strdup, no ObjC retain) and signals; the response is rebuilt off-thread from our own bytes, using arrays + a mutable dictionary to avoid the single-entry immutable dictionaries involved in the over-release. Verified on a real iOS 26.5 device: list, eval (strings/arrays/200K string), repeated content (full HTML), and goto all succeed with the app staying alive.
…/agent.m The injected LLDB expression was a ~17 KB / 238-line Go raw-string literal — roughly half of ios_device_webview.go. Extract it to agents/ios-real/agent.m and embed it from the agents package (same convention as the simulator dylib and Android agent), referenced as agents.IOSRealDeviceWebViewAgent. The .m lives under agents/ (not devices/) because a Go package may not contain C/.m source files without cgo, and //go:embed cannot reach across "..". It is an LLDB expression, not a standalone translation unit, so it is embedded, not compiled — the .m extension is just for editor highlighting. A leading newline is prepended after "expr -l objc --" so LLDB enters multi-line expression mode and treats the whole source as one expression (the original raw-string literal began with a newline; the embedded file does not).
The device agent hard-coded bounds to zero. Compute the webview's frame in
window coordinates (matching the simulator agent's
convertRect:bounds toView:nil). UIKit/CoreGraphics headers aren't on the LLDB
expression's include path, so call the struct-returning -bounds and
-convertRect:toView: through cast'd objc_msgSend with a CGRect-compatible
struct (CGFloat is double on arm64; arm64 has no objc_msgSend_stret).
Verified on a real iOS 26.5 device: bounds now report e.g.
{x:0, y:102, width:414, height:794}.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR introduces real iOS device WebView automation by implementing an end-to-end stack: a GDB Remote Serial Protocol handler for device debugserver communication, a device-side Objective-C TCP server agent that exposes WebView operations via JSON-RPC, LLDB infrastructure to inject the agent and tunnel GDB RSP traffic, and host-side orchestration methods that manage agent lifecycle with port caching and reuse. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agents/ios-real/agent.m`:
- Around line 173-206: The handler currently ignores params[@"args"] when
evaluating expressions; either consume those args or return an explicit error.
Update the block that builds `wrapped` (where `expr` is used) to read `id args =
params[@"args"]` and if args is present JSON-encode it and inject it into the JS
invocation (e.g. call the expression as a function with the parsed args) so the
JS receives the parameters, or if you prefer to disallow args, set `resp` to an
error JSON (similar to the missing id/expr branch) when args != nil; touch the
symbols `params`, `expr`, `wrapped`, and the evaluateJavaScript completion flow
so the injected args are sent safely (or explicitly rejected) before calling
`evaluateJavaScript`.
- Around line 241-244: The current sends for hdrData and resp may short-write
and ignore return values; update the send logic around hdrs/hdrData and resp to
loop until all bytes are written: for each buffer (hdrData.bytes/hdrData.length
and resp.bytes/resp.length) call send(cfd, ptr, remaining, 0), check the ssize_t
return, on positive advance ptr by bytes_sent and decrement remaining, on 0 or
-1 handle EINTR by retrying and treat other errors as fatal (close cfd/return);
ensure you use unsigned char* pointer arithmetic and preserve the original
buffers (hdrs/hdrData and resp) so the full HTTP header + JSON body is
transmitted.
In `@devices/ios_device_webview.go`:
- Around line 399-415: findRunningDeviceAgent currently reuses any responsive
agent on device ports 27042–27051 without verifying ownership, causing cross-app
hits; update the flow so that after establishing the port forward
(iosutil.NewPortForwarder) and before returning, perform an identity handshake
with the agent (extend isAgentReady or add a new helper like
getAgentIdentity(localPort) to return bundle ID or PID) and compare that
identity to the current foreground app (use or add a helper such as
getForegroundAppBundleID on IOSDevice or equivalent); only return localPort if
the identities match, otherwise call pf.Stop() and continue scanning, ensuring
no leftover forwards are reused for the wrong app.
- Around line 168-177: The code treats a PID as foreground because state
defaults to 0 and the error from rt.Call is ignored; change the logic to capture
and check the error from the applicationState call separately (e.g., appInst,
err := rt.ClassCall("UIApplication", "sharedApplication"); if err == nil {
state, callErr := rt.Call(appInst, "applicationState"); if callErr != nil { /*
mark state unknown / skip foreground logic */ } } ), only use state when the
rt.Call returned no error, and ensure the foreground branch (the condition using
state) requires both no errors and state == 0 (referencing appInst,
rt.ClassCall, rt.Call, and the state variable).
In `@devices/ios/debuggertools/macho.go`:
- Around line 258-267: The code drops errors during symbol bootstrap: capture
and handle errors returned by the first gdb.Request("qShlibInfoAddr") call,
validate fmt.Sscanf into shlibAddr (check its returned count), and
capture/handle the error from the second gdb.Request(fmt.Sprintf("m%x,%x",
shlibAddr, imageInfoArrayOff+8)) as well as the hex.DecodeString(resp) error
before proceeding; return a clear fmt.Errorf from the surrounding function on
any of these failures so symbol bootstrapping fails fast and reports the
underlying transport/parse/decode error (referencing gdb.Request,
qShlibInfoAddr, shlibAddr, fmt.Sscanf, imageInfoArrayOff, and hex.DecodeString).
- Around line 126-128: The calculation of trieAddr uses unsigned subtraction
that can underflow (uint64(exportFileOff) - linkeditFileOff); guard this by
validating offsets before computing trieAddr: ensure exportFileOff (converted to
uint64) is >= linkeditFileOff (or perform signed check) and that the resulting
delta is within the linkedit size bounds; if the check fails, return an error or
skip processing. Locate the trieAddr computation (the slide, linkeditVMAddr,
exportFileOff and linkeditFileOff variables) and add the conditional check and
appropriate error handling to avoid producing a bogus address.
In `@devices/ios/debuggertools/objc_runtime.go`:
- Around line 253-263: The writeData method currently doesn't verify that the
aligned write fits inside the single data page; before issuing the GDB write,
compute required := (uint64(len(data)) + 7) &^ 7 and check if m.dataOff+required
<= m.dataPageSize, and if not either allocate a fresh data page (implement a
small helper like m.allocDataPage() that updates m.dataAddr and resets
m.dataOff) or return a clear error; then proceed to call Request and update
m.dataOff by required (not recomputing alignment afterward). Ensure you
reference gdbMem.writeData, m.dataAddr, m.dataOff, and m.dataPageSize when
making the change.
- Around line 156-173: The Call method in ObjCRuntime currently treats a zero
return from objc_msgSend as an error; remove the post-call nil-check so valid
zero/false/nil returns (e.g., UIApplication.applicationState == 0) are not
surfaced as failures. Specifically, in ObjCRuntime.Call, after calling
rt.mem.call(rt.msgSend, ...) do not return an error when result == 0 — only
propagate the error returned by rt.mem.call; delete or disable the block that
does "if result == 0 { return 0, fmt.Errorf(...)" so callers can interpret a
zero result themselves.
In `@devices/ios/debugserver/gdbserver.go`:
- Around line 80-85: The Recv method on GDBServer currently treats a closed
scanner as a successful empty payload; change Recv so that when g.scanner.Scan()
returns false you check g.scanner.Err() and if that error is nil return io.EOF
(instead of nil error), otherwise return the scanner error; update imports to
include io if not present. This affects the Recv function and uses g.scanner and
g.scanner.Err() as the identifying symbols.
- Around line 40-60: The split function passed to scanner.Split currently
returns the bytes between '$' and '#' without verifying the two-byte hex
checksum; update that anonymous func to compute the modulo-256 checksum of
data[start+1:end], parse the two hex digits immediately after '#'
(data[end+1:end+3]), and compare them; if the checksum does not match, return 0,
nil, ErrInvalidGDBServerPayload (or another appropriate error) instead of
accepting the packet so corrupted frames are rejected before further parsing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 48ae61b4-5355-4e30-aba0-e78b90311d88
📒 Files selected for processing (8)
agents/agents.goagents/ios-real/agent.mdevices/ios/debuggertools/LICENSEdevices/ios/debuggertools/macho.godevices/ios/debuggertools/objc_runtime.godevices/ios/debugserver/LICENSEdevices/ios/debugserver/gdbserver.godevices/ios_device_webview.go
Comment on lines
+173
to
+206
| NSString *wvId = params[@"id"], *expr = params[@"expression"]; | ||
| id wv = (wvId && expr) ? __findWV(wvId) : nil; | ||
| if (!wvId || !expr) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32602),@"message":@"missing id or expression"}} options:0 error:nil]; | ||
| else if (!wv) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":@"webview not found"}} options:0 error:nil]; | ||
| else { | ||
| // A heap (non-tagged) value returned from evaluateJavaScript is | ||
| // over-released when WebKit's delivery pool drains, crashing the | ||
| // app — so we must never hold WebKit's result object. Instead the | ||
| // JS JSON-stringifies [ok, value]; inside the handler we copy the | ||
| // resulting bytes to C memory (no ObjC retain) and rebuild the | ||
| // response from our own bytes. Arrays + a mutable dict avoid the | ||
| // single-entry immutable dictionaries seen in the crash. | ||
| NSString *wrapped = [NSString stringWithFormat:@"(function(){try{return JSON.stringify([1,(function(){%@})()])}catch(e){return JSON.stringify([0,''+e])}})()", expr]; | ||
| __block char *jbuf = NULL; | ||
| id sem = dispatch_semaphore_create(0); | ||
| [[NSOperationQueue mainQueue] addOperationWithBlock:^{ | ||
| (void)[wv evaluateJavaScript:wrapped completionHandler:^(id r, NSError *e) { | ||
| if ([(NSObject *)r isKindOfClass:[NSString class]]) { const char *u = [(NSString *)r UTF8String]; if (u) jbuf = strdup(u); } | ||
| dispatch_semaphore_signal(sem); | ||
| }]; | ||
| }]; | ||
| dispatch_semaphore_wait(sem, dispatch_time(0, 30000000000LL)); | ||
| if (!jbuf) { | ||
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":@"no result from evaluate"}} options:0 error:nil]; | ||
| } else { | ||
| NSArray *parsed = [NSJSONSerialization JSONObjectWithData:[NSData dataWithBytes:jbuf length:strlen(jbuf)] options:0 error:nil]; | ||
| free(jbuf); | ||
| BOOL ok2 = [(NSObject *)parsed isKindOfClass:[NSArray class]] && [parsed count] == 2; | ||
| if (ok2 && [(NSNumber *)parsed[0] intValue] == 0) { | ||
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":parsed[1]}} options:0 error:nil]; | ||
| } else { | ||
| NSMutableDictionary *rd = [NSMutableDictionary dictionary]; | ||
| rd[@"result"] = ok2 ? parsed[1] : [NSNull null]; | ||
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"result":rd} options:0 error:nil]; |
There was a problem hiding this comment.
Honor args in device.webview.evaluate or reject them explicitly.
IOSDevice.WebViewEvaluate sends params["args"], but this handler never reads them. Any caller relying on parameterized evaluation gets incorrect behavior with no error.
Minimal safe fix
- NSString *wvId = params[@"id"], *expr = params[@"expression"];
+ NSString *wvId = params[@"id"], *expr = params[@"expression"];
+ NSArray *args = params[@"args"];
id wv = (wvId && expr) ? __findWV(wvId) : nil;
if (!wvId || !expr) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32602),@"message":@"missing id or expression"}} options:0 error:nil];
+ else if ([args isKindOfClass:[NSArray class]] && [args count] > 0) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32602),@"message":@"args not supported on real iOS yet"}} options:0 error:nil];
else if (!wv) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":@"webview not found"}} options:0 error:nil];
else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| NSString *wvId = params[@"id"], *expr = params[@"expression"]; | |
| id wv = (wvId && expr) ? __findWV(wvId) : nil; | |
| if (!wvId || !expr) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32602),@"message":@"missing id or expression"}} options:0 error:nil]; | |
| else if (!wv) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":@"webview not found"}} options:0 error:nil]; | |
| else { | |
| // A heap (non-tagged) value returned from evaluateJavaScript is | |
| // over-released when WebKit's delivery pool drains, crashing the | |
| // app — so we must never hold WebKit's result object. Instead the | |
| // JS JSON-stringifies [ok, value]; inside the handler we copy the | |
| // resulting bytes to C memory (no ObjC retain) and rebuild the | |
| // response from our own bytes. Arrays + a mutable dict avoid the | |
| // single-entry immutable dictionaries seen in the crash. | |
| NSString *wrapped = [NSString stringWithFormat:@"(function(){try{return JSON.stringify([1,(function(){%@})()])}catch(e){return JSON.stringify([0,''+e])}})()", expr]; | |
| __block char *jbuf = NULL; | |
| id sem = dispatch_semaphore_create(0); | |
| [[NSOperationQueue mainQueue] addOperationWithBlock:^{ | |
| (void)[wv evaluateJavaScript:wrapped completionHandler:^(id r, NSError *e) { | |
| if ([(NSObject *)r isKindOfClass:[NSString class]]) { const char *u = [(NSString *)r UTF8String]; if (u) jbuf = strdup(u); } | |
| dispatch_semaphore_signal(sem); | |
| }]; | |
| }]; | |
| dispatch_semaphore_wait(sem, dispatch_time(0, 30000000000LL)); | |
| if (!jbuf) { | |
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":@"no result from evaluate"}} options:0 error:nil]; | |
| } else { | |
| NSArray *parsed = [NSJSONSerialization JSONObjectWithData:[NSData dataWithBytes:jbuf length:strlen(jbuf)] options:0 error:nil]; | |
| free(jbuf); | |
| BOOL ok2 = [(NSObject *)parsed isKindOfClass:[NSArray class]] && [parsed count] == 2; | |
| if (ok2 && [(NSNumber *)parsed[0] intValue] == 0) { | |
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":parsed[1]}} options:0 error:nil]; | |
| } else { | |
| NSMutableDictionary *rd = [NSMutableDictionary dictionary]; | |
| rd[@"result"] = ok2 ? parsed[1] : [NSNull null]; | |
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"result":rd} options:0 error:nil]; | |
| NSString *wvId = params[@"id"], *expr = params[@"expression"]; | |
| NSArray *args = params[@"args"]; | |
| id wv = (wvId && expr) ? __findWV(wvId) : nil; | |
| if (!wvId || !expr) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32602),@"message":@"missing id or expression"}} options:0 error:nil]; | |
| else if ([args isKindOfClass:[NSArray class]] && [args count] > 0) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32602),@"message":@"args not supported on real iOS yet"}} options:0 error:nil]; | |
| else if (!wv) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":@"webview not found"}} options:0 error:nil]; | |
| else { | |
| // A heap (non-tagged) value returned from evaluateJavaScript is | |
| // over-released when WebKit's delivery pool drains, crashing the | |
| // app — so we must never hold WebKit's result object. Instead the | |
| // JS JSON-stringifies [ok, value]; inside the handler we copy the | |
| // resulting bytes to C memory (no ObjC retain) and rebuild the | |
| // response from our own bytes. Arrays + a mutable dict avoid the | |
| // single-entry immutable dictionaries seen in the crash. | |
| NSString *wrapped = [NSString stringWithFormat:@"(function(){try{return JSON.stringify([1,(function(){%@})()])}catch(e){return JSON.stringify([0,''+e])}})()", expr]; | |
| __block char *jbuf = NULL; | |
| id sem = dispatch_semaphore_create(0); | |
| [[NSOperationQueue mainQueue] addOperationWithBlock:^{ | |
| (void)[wv evaluateJavaScript:wrapped completionHandler:^(id r, NSError *e) { | |
| if ([(NSObject *)r isKindOfClass:[NSString class]]) { const char *u = [(NSString *)r UTF8String]; if (u) jbuf = strdup(u); } | |
| dispatch_semaphore_signal(sem); | |
| }]; | |
| }]; | |
| dispatch_semaphore_wait(sem, dispatch_time(0, 30000000000LL)); | |
| if (!jbuf) { | |
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":@"no result from evaluate"}} options:0 error:nil]; | |
| } else { | |
| NSArray *parsed = [NSJSONSerialization JSONObjectWithData:[NSData dataWithBytes:jbuf length:strlen(jbuf)] options:0 error:nil]; | |
| free(jbuf); | |
| BOOL ok2 = [(NSObject *)parsed isKindOfClass:[NSArray class]] && [parsed count] == 2; | |
| if (ok2 && [(NSNumber *)parsed[0] intValue] == 0) { | |
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":parsed[1]}} options:0 error:nil]; | |
| } else { | |
| NSMutableDictionary *rd = [NSMutableDictionary dictionary]; | |
| rd[@"result"] = ok2 ? parsed[1] : [NSNull null]; | |
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"result":rd} options:0 error:nil]; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agents/ios-real/agent.m` around lines 173 - 206, The handler currently
ignores params[@"args"] when evaluating expressions; either consume those args
or return an explicit error. Update the block that builds `wrapped` (where
`expr` is used) to read `id args = params[@"args"]` and if args is present
JSON-encode it and inject it into the JS invocation (e.g. call the expression as
a function with the parsed args) so the JS receives the parameters, or if you
prefer to disallow args, set `resp` to an error JSON (similar to the missing
id/expr branch) when args != nil; touch the symbols `params`, `expr`, `wrapped`,
and the evaluateJavaScript completion flow so the injected args are sent safely
(or explicitly rejected) before calling `evaluateJavaScript`.
Comment on lines
+126
to
+128
| slide := baseAddr - textVMAddr | ||
| trieAddr := slide + linkeditVMAddr + (uint64(exportFileOff) - linkeditFileOff) | ||
|
|
There was a problem hiding this comment.
Guard unsigned arithmetic before computing trieAddr.
On Line 127, uint64(exportFileOff) - linkeditFileOff can underflow if offsets are inconsistent/malformed, yielding a huge bogus address.
Suggested fix
- slide := baseAddr - textVMAddr
- trieAddr := slide + linkeditVMAddr + (uint64(exportFileOff) - linkeditFileOff)
+ if baseAddr < textVMAddr || uint64(exportFileOff) < linkeditFileOff {
+ return 0
+ }
+ slide := baseAddr - textVMAddr
+ trieAddr := slide + linkeditVMAddr + (uint64(exportFileOff) - linkeditFileOff)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| slide := baseAddr - textVMAddr | |
| trieAddr := slide + linkeditVMAddr + (uint64(exportFileOff) - linkeditFileOff) | |
| if baseAddr < textVMAddr || uint64(exportFileOff) < linkeditFileOff { | |
| return 0 | |
| } | |
| slide := baseAddr - textVMAddr | |
| trieAddr := slide + linkeditVMAddr + (uint64(exportFileOff) - linkeditFileOff) | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devices/ios/debuggertools/macho.go` around lines 126 - 128, The calculation
of trieAddr uses unsigned subtraction that can underflow (uint64(exportFileOff)
- linkeditFileOff); guard this by validating offsets before computing trieAddr:
ensure exportFileOff (converted to uint64) is >= linkeditFileOff (or perform
signed check) and that the resulting delta is within the linkedit size bounds;
if the check fails, return an error or skip processing. Locate the trieAddr
computation (the slide, linkeditVMAddr, exportFileOff and linkeditFileOff
variables) and add the conditional check and appropriate error handling to avoid
producing a bogus address.
Comment on lines
+258
to
+267
| resp, _ := gdb.Request("qShlibInfoAddr") | ||
| if resp == "" || strings.HasPrefix(resp, "E") { | ||
| return nil, fmt.Errorf("qShlibInfoAddr: %s", resp) | ||
| } | ||
| var shlibAddr uint64 | ||
| fmt.Sscanf(resp, "%x", &shlibAddr) | ||
|
|
||
| // Read version + count + infoArray pointer | ||
| resp, _ = gdb.Request(fmt.Sprintf("m%x,%x", shlibAddr, imageInfoArrayOff+8)) | ||
| shlibData, _ := hex.DecodeString(resp) |
There was a problem hiding this comment.
Don’t drop transport/parse/decode errors in symbol bootstrap.
Lines 258-267 ignore gdb.Request errors, Sscanf result, and hex.DecodeString errors. Failures here get misreported later as symbol misses.
Suggested fix
- resp, _ := gdb.Request("qShlibInfoAddr")
+ resp, err := gdb.Request("qShlibInfoAddr")
+ if err != nil {
+ return nil, fmt.Errorf("qShlibInfoAddr request failed: %w", err)
+ }
if resp == "" || strings.HasPrefix(resp, "E") {
return nil, fmt.Errorf("qShlibInfoAddr: %s", resp)
}
var shlibAddr uint64
- fmt.Sscanf(resp, "%x", &shlibAddr)
+ if n, scanErr := fmt.Sscanf(resp, "%x", &shlibAddr); scanErr != nil || n != 1 {
+ return nil, fmt.Errorf("invalid qShlibInfoAddr response: %q", resp)
+ }
// Read version + count + infoArray pointer
- resp, _ = gdb.Request(fmt.Sprintf("m%x,%x", shlibAddr, imageInfoArrayOff+8))
- shlibData, _ := hex.DecodeString(resp)
+ resp, err = gdb.Request(fmt.Sprintf("m%x,%x", shlibAddr, imageInfoArrayOff+8))
+ if err != nil {
+ return nil, fmt.Errorf("read dyld_all_image_infos failed: %w", err)
+ }
+ shlibData, err := hex.DecodeString(resp)
+ if err != nil {
+ return nil, fmt.Errorf("invalid dyld_all_image_infos hex payload: %w", err)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| resp, _ := gdb.Request("qShlibInfoAddr") | |
| if resp == "" || strings.HasPrefix(resp, "E") { | |
| return nil, fmt.Errorf("qShlibInfoAddr: %s", resp) | |
| } | |
| var shlibAddr uint64 | |
| fmt.Sscanf(resp, "%x", &shlibAddr) | |
| // Read version + count + infoArray pointer | |
| resp, _ = gdb.Request(fmt.Sprintf("m%x,%x", shlibAddr, imageInfoArrayOff+8)) | |
| shlibData, _ := hex.DecodeString(resp) | |
| resp, err := gdb.Request("qShlibInfoAddr") | |
| if err != nil { | |
| return nil, fmt.Errorf("qShlibInfoAddr request failed: %w", err) | |
| } | |
| if resp == "" || strings.HasPrefix(resp, "E") { | |
| return nil, fmt.Errorf("qShlibInfoAddr: %s", resp) | |
| } | |
| var shlibAddr uint64 | |
| if n, scanErr := fmt.Sscanf(resp, "%x", &shlibAddr); scanErr != nil || n != 1 { | |
| return nil, fmt.Errorf("invalid qShlibInfoAddr response: %q", resp) | |
| } | |
| // Read version + count + infoArray pointer | |
| resp, err = gdb.Request(fmt.Sprintf("m%x,%x", shlibAddr, imageInfoArrayOff+8)) | |
| if err != nil { | |
| return nil, fmt.Errorf("read dyld_all_image_infos failed: %w", err) | |
| } | |
| shlibData, err := hex.DecodeString(resp) | |
| if err != nil { | |
| return nil, fmt.Errorf("invalid dyld_all_image_infos hex payload: %w", err) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devices/ios/debuggertools/macho.go` around lines 258 - 267, The code drops
errors during symbol bootstrap: capture and handle errors returned by the first
gdb.Request("qShlibInfoAddr") call, validate fmt.Sscanf into shlibAddr (check
its returned count), and capture/handle the error from the second
gdb.Request(fmt.Sprintf("m%x,%x", shlibAddr, imageInfoArrayOff+8)) as well as
the hex.DecodeString(resp) error before proceeding; return a clear fmt.Errorf
from the surrounding function on any of these failures so symbol bootstrapping
fails fast and reports the underlying transport/parse/decode error (referencing
gdb.Request, qShlibInfoAddr, shlibAddr, fmt.Sscanf, imageInfoArrayOff, and
hex.DecodeString).
Comment on lines
+156
to
+173
| func (rt *ObjCRuntime) Call(receiver uint64, selector string, args ...uint64) (uint64, error) { | ||
| if receiver == 0 { | ||
| return 0, fmt.Errorf("nil receiver for [? %s]", selector) | ||
| } | ||
| sel, err := rt.sel(selector) | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
| callArgs := make([]uint64, 0, 2+len(args)) | ||
| callArgs = append(callArgs, receiver, sel) | ||
| callArgs = append(callArgs, args...) | ||
| result, err := rt.mem.call(rt.msgSend, callArgs...) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("objc_msgSend(%s): %w", selector, err) | ||
| } | ||
| if result == 0 { | ||
| return 0, fmt.Errorf("[0x%x %s] returned nil", receiver, selector) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Objective-C on iOS, can methods invoked via objc_msgSendlegitimately return0/NO/nilwithout that meaning a transport failure, and what numeric value doesUIApplication.applicationState use for the active state?
💡 Result:
In Objective-C, methods invoked via objc_msgSend can legitimately return 0, NO, or nil without it representing a transport failure [1]. This behavior is a fundamental design feature of the Objective-C runtime: sending a message to a nil receiver does not cause a crash or a transport error; instead, it is treated as a no-op that returns 0 (for scalar or pointer return types) [2][1]. Because the runtime simply returns the zeroed-out value when the receiver is nil, this result is indistinguishable from a method that intentionally returns 0, NO, or nil [3][1]. Regarding UIApplication.applicationState, the active state is represented by the enum constant UIApplicationStateActive (or UIApplication.State.active in Swift) [4][5]. As an enumeration, this value uses an underlying integer, which is 0 [5]. Summary of return behavior for messages sent to nil: - Objects: Returns nil (0) [1]. - Scalar values (ints, chars, etc.): Returns 0 [3][1]. - Structs: Returns 0.0 for every field if the struct is returned in registers; otherwise, the result is undefined [1].
Citations:
- 1: https://stackoverflow.com/questions/2696891/calling-a-method-on-an-uninitialized-object-null-pointer
- 2: https://www.mikeash.com/pyblog/friday-qa-2012-12-14-objective-c-pitfalls.html
- 3: https://ridiculousfish.com/blog/posts/nil.html
- 4: https://developer.apple.com/documentation/uikit/uiapplication/state
- 5: https://stackoverflow.com/questions/7937400/need-clarification-about-uiapplicationstate
Don’t treat a zero objc_msgSend return as a generic failure in ObjCRuntime.Call.
In devices/ios/debuggertools/objc_runtime.go (around the if result == 0 check), Objective-C message sends can legitimately return 0/NO/nil without indicating a transport/runtime failure. Specifically, UIApplication.applicationState’s active state (UIApplicationStateActive) is the enum value 0, so this layer currently turns a valid “active” result into an error.
Suggested fix
func (rt *ObjCRuntime) Call(receiver uint64, selector string, args ...uint64) (uint64, error) {
if receiver == 0 {
return 0, fmt.Errorf("nil receiver for [? %s]", selector)
}
sel, err := rt.sel(selector)
if err != nil {
return 0, err
}
callArgs := make([]uint64, 0, 2+len(args))
callArgs = append(callArgs, receiver, sel)
callArgs = append(callArgs, args...)
result, err := rt.mem.call(rt.msgSend, callArgs...)
if err != nil {
return 0, fmt.Errorf("objc_msgSend(%s): %w", selector, err)
}
- if result == 0 {
- return 0, fmt.Errorf("[0x%x %s] returned nil", receiver, selector)
- }
return result, nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func (rt *ObjCRuntime) Call(receiver uint64, selector string, args ...uint64) (uint64, error) { | |
| if receiver == 0 { | |
| return 0, fmt.Errorf("nil receiver for [? %s]", selector) | |
| } | |
| sel, err := rt.sel(selector) | |
| if err != nil { | |
| return 0, err | |
| } | |
| callArgs := make([]uint64, 0, 2+len(args)) | |
| callArgs = append(callArgs, receiver, sel) | |
| callArgs = append(callArgs, args...) | |
| result, err := rt.mem.call(rt.msgSend, callArgs...) | |
| if err != nil { | |
| return 0, fmt.Errorf("objc_msgSend(%s): %w", selector, err) | |
| } | |
| if result == 0 { | |
| return 0, fmt.Errorf("[0x%x %s] returned nil", receiver, selector) | |
| } | |
| func (rt *ObjCRuntime) Call(receiver uint64, selector string, args ...uint64) (uint64, error) { | |
| if receiver == 0 { | |
| return 0, fmt.Errorf("nil receiver for [? %s]", selector) | |
| } | |
| sel, err := rt.sel(selector) | |
| if err != nil { | |
| return 0, err | |
| } | |
| callArgs := make([]uint64, 0, 2+len(args)) | |
| callArgs = append(callArgs, receiver, sel) | |
| callArgs = append(callArgs, args...) | |
| result, err := rt.mem.call(rt.msgSend, callArgs...) | |
| if err != nil { | |
| return 0, fmt.Errorf("objc_msgSend(%s): %w", selector, err) | |
| } | |
| return result, nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devices/ios/debuggertools/objc_runtime.go` around lines 156 - 173, The Call
method in ObjCRuntime currently treats a zero return from objc_msgSend as an
error; remove the post-call nil-check so valid zero/false/nil returns (e.g.,
UIApplication.applicationState == 0) are not surfaced as failures. Specifically,
in ObjCRuntime.Call, after calling rt.mem.call(rt.msgSend, ...) do not return an
error when result == 0 — only propagate the error returned by rt.mem.call;
delete or disable the block that does "if result == 0 { return 0,
fmt.Errorf(...)" so callers can interpret a zero result themselves.
Comment on lines
+40
to
+60
| scanner.Split(func(data []byte, atEOF bool) (int, []byte, error) { | ||
| const lenPacketSuffix = 3 // len("#00") | ||
|
|
||
| if atEOF && len(data) == 0 { | ||
| return 0, nil, nil | ||
| } | ||
|
|
||
| start := bytes.IndexRune(data, '$') | ||
| end := bytes.IndexRune(data, '#') | ||
| // Need more data | ||
| if start < 0 || end < 0 || len(data) < end+lenPacketSuffix { | ||
| return 0, nil, nil | ||
| } | ||
| // Invalid data | ||
| if end < start { | ||
| return 0, nil, ErrInvalidGDBServerPayload | ||
| } | ||
|
|
||
| // Strip the $ prefix before returning | ||
| return end + lenPacketSuffix, data[start+1 : end], nil | ||
| }) |
There was a problem hiding this comment.
Validate RSP checksum before accepting a packet.
On Line 59, the split function returns payload bytes without checking the #xx checksum. A corrupted frame is treated as valid and can cascade into wrong memory/symbol parsing.
Suggested fix
scanner.Split(func(data []byte, atEOF bool) (int, []byte, error) {
const lenPacketSuffix = 3 // len("`#00`")
@@
- // Strip the $ prefix before returning
- return end + lenPacketSuffix, data[start+1 : end], nil
+ payload := data[start+1 : end]
+ recvChecksumHex := data[end+1 : end+3]
+ recvChecksum, err := hex.DecodeString(string(recvChecksumHex))
+ if err != nil || len(recvChecksum) != 1 {
+ return 0, nil, ErrInvalidGDBServerPayload
+ }
+ sum := 0
+ for _, b := range payload {
+ sum += int(b)
+ }
+ if recvChecksum[0] != byte(sum%256) {
+ return 0, nil, ErrInvalidGDBServerPayload
+ }
+
+ // Strip the $ prefix before returning
+ return end + lenPacketSuffix, payload, nil
})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devices/ios/debugserver/gdbserver.go` around lines 40 - 60, The split
function passed to scanner.Split currently returns the bytes between '$' and '#'
without verifying the two-byte hex checksum; update that anonymous func to
compute the modulo-256 checksum of data[start+1:end], parse the two hex digits
immediately after '#' (data[end+1:end+3]), and compare them; if the checksum
does not match, return 0, nil, ErrInvalidGDBServerPayload (or another
appropriate error) instead of accepting the packet so corrupted frames are
rejected before further parsing.
Comment on lines
+80
to
+85
| func (g *GDBServer) Recv() (string, error) { | ||
| if g.scanner.Scan() == false { | ||
| return "", g.scanner.Err() | ||
| } | ||
| return g.scanner.Text(), nil | ||
| } |
There was a problem hiding this comment.
Return io.EOF when the stream closes.
On Lines 81-83, Scan()==false with no scanner error returns ("", nil). That hides connection closure as a successful empty payload.
Suggested fix
func (g *GDBServer) Recv() (string, error) {
- if g.scanner.Scan() == false {
- return "", g.scanner.Err()
- }
+ if g.scanner.Scan() == false {
+ if err := g.scanner.Err(); err != nil {
+ return "", err
+ }
+ return "", io.EOF
+ }
return g.scanner.Text(), nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func (g *GDBServer) Recv() (string, error) { | |
| if g.scanner.Scan() == false { | |
| return "", g.scanner.Err() | |
| } | |
| return g.scanner.Text(), nil | |
| } | |
| func (g *GDBServer) Recv() (string, error) { | |
| if g.scanner.Scan() == false { | |
| if err := g.scanner.Err(); err != nil { | |
| return "", err | |
| } | |
| return "", io.EOF | |
| } | |
| return g.scanner.Text(), nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devices/ios/debugserver/gdbserver.go` around lines 80 - 85, The Recv method
on GDBServer currently treats a closed scanner as a successful empty payload;
change Recv so that when g.scanner.Scan() returns false you check
g.scanner.Err() and if that error is nil return io.EOF (instead of nil error),
otherwise return the scanner error; update imports to include io if not present.
This affects the Recv function and uses g.scanner and g.scanner.Err() as the
identifying symbols.
The injected agent scanned 27042-27051 for a free port and the reuse probe scanned the same range. Bind a single fixed port (12008) instead: it removes the scan on both the agent and host sides and keeps reuse discovery trivial (the simulator path avoids this differently, via an exported mobilecli_get_port symbol, which an injected expression can't offer). Also quieten --verbose: log lldb-proxy device responses as a byte count rather than dumping the (often binary) packet contents. Verified on a real iOS 26.5 device: fresh inject binds 12008, the reuse fast-path finds it, and list/content work.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agents/ios-real/agent.m`:
- Around line 50-51: The code currently binds __sfd to a fixed port
(htons(12008)) and on bind failure it leaves the socket open; update the failure
path in the bind block to call close(__sfd) and set __sfd to -1 (or another
sentinel) so the descriptor is not leaked, and only set __port = 12008 when bind
returns success; modify the block that calls bind(__sfd, (struct __sockaddr
*)&__sa, sizeof(__sa)) to perform the close(__sfd) cleanup on the non-zero
return path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: befdfd7b-ddf9-4218-bf83-06ade2d68468
📒 Files selected for processing (2)
agents/ios-real/agent.mdevices/ios_device_webview.go
🚧 Files skipped from review as they are similar to previous changes (1)
- devices/ios_device_webview.go
Only the device-side agent port needs to be fixed (12008, for cross-run reuse discovery); the local end of the tunnel forward is purely in-process. Replace the findFreeLocalPort range scan with a single ":0" bind so the kernel assigns the local port (go-ios's forward.Forward rejects a literal port 0, so we grab a free one via net.Listen and pass it). Verified on a real iOS device: agent binds device:12008, host forwards an ephemeral local port (e.g. 65135) to it; list/content work.
Replace the bespoke no-WDA foreground detection (attach to every running user app via the debug proxy and read UIApplicationState through an ObjC-runtime- over-GDB bridge) with WDA's GetActiveAppInfo, which returns the foreground bundleID and pid directly. ensureIOSDeviceAgentReady now calls StartAgent to bring WDA up (tunnel + :8100 forward + launch) before the lookup. Much more stable (the attach-each-app probe had no timeout and could hang on a misbehaving app — observed in testing), and removes a lot of code: - delete findForegroundApp, userApps, and the userApp type - delete the vendored devices/ios/debuggertools package (its only user) - drop the installationproxy / instruments / debuggertools imports debugserver stays (the LLDB proxy bridge still uses GDBServer). Verified on a real iOS 26.5 device: WDA launches, foreground app is detected, agent injects, list/content work.
The injected agent called send() once for the header and once for the body and ignored the return values, so a short write (e.g. interrupted by a signal) would silently truncate the response. Loop until all bytes are written, retrying on EINTR and treating other errors as fatal, advancing through each buffer via unsigned char* arithmetic.
ios_device_webview.go had grown to mix three responsibilities; with view-tree /
network-capture / image-injection features planned on the same injected agent,
separate them so each future feature is a thin sibling of webview rather than
more weight on one file:
- ios_device_lldbproxy.go — the GDB-RSP bridge (startLLDBProxy, lldbProxyConn)
- ios_device_agent.go — inject + session: injection, port cache, reuse probe,
ensureIOSDeviceAgentReady (now a thin orchestrator
over injectFreshAgent/forwardToAgent), and the
agentCall seam every feature uses
- ios_device_webview.go — only the WebViewable methods, now one-liners
agentCall/agentCallWithTimeout centralise "ensure agent ready -> request ->
drop cached port on failure", which also fixes inconsistent cache invalidation
(previously only ListWebViews reset the cache on a failed call, so a dead agent
would make goto/evaluate/etc. fail repeatedly until a list re-injected it).
No behaviour change; build + devices tests pass.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@devices/ios_device_agent.go`:
- Around line 77-80: The code currently only caches the local port via
setCachedDeviceAgentPort(d.Udid, ...) when using agentRequestWithTimeout, which
leaves forwarder goroutines/listeners orphaned; change the cache to store the
forwarder lifecycle (e.g., a forwarder object or cancel/Close function) instead
of just the port, return and use that forwarder from the injection/forwarding
logic, and on any failure paths (the error branch after agentRequestWithTimeout
and the other similar sites around agent injection) call
forwarder.Stop()/Close() and replace the cached forwarder atomically when
creating a new one so previous forwarders are explicitly stopped and not
leaked—update related helpers that read/write the cache to handle the new
forwarder type and ensure setCachedDeviceAgentPort usage is replaced with
setCachedDeviceAgentForwarder (or equivalent) in functions like
agentRequestWithTimeout call sites, injection/retry logic, and cleanup paths.
- Around line 88-108: Concurrent calls to ensureIOSDeviceAgentReady can race
past the cachedDeviceAgentPort check and both call
findRunningDeviceAgent/injectFreshAgent; serialize the whole setup per device by
adding a per-UDID lock and holding it for the duration of
ensureIOSDeviceAgentReady. Modify ensureIOSDeviceAgentReady to acquire a mutex
for d.Udid (e.g., add a sync.Mutex in IOSDevice or use a package-level
map[udid]*sync.Mutex) before checking cachedDeviceAgentPort and release it at
the end, ensuring the startTunnel, findRunningDeviceAgent, injectFreshAgent and
setCachedDeviceAgentPort sequence is executed under that lock so only one
goroutine performs agent setup for a given UDID.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e6117028-d0a3-43b5-9e0c-e93c59f3b9ea
📒 Files selected for processing (4)
agents/ios-real/agent.mdevices/ios_device_agent.godevices/ios_device_lldbproxy.godevices/ios_device_webview.go
🚧 Files skipped from review as they are similar to previous changes (1)
- agents/ios-real/agent.m
Comment on lines
+77
to
+80
| raw, err := agentRequestWithTimeout(port, method, params, timeout) | ||
| if err != nil { | ||
| // the agent may have died; drop the cached port so we re-inject next time | ||
| setCachedDeviceAgentPort(d.Udid, 0) |
There was a problem hiding this comment.
Cache the forwarder lifecycle, not just the local port.
Successful forwards survive past the call site, but the cache only remembers the port number. When a request fails or a new injection replaces the old one, the previous forwarder is never stopped, so retries can accumulate orphaned listeners/goroutines for the same device.
Also applies to: 98-108, 120-135, 186-206
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devices/ios_device_agent.go` around lines 77 - 80, The code currently only
caches the local port via setCachedDeviceAgentPort(d.Udid, ...) when using
agentRequestWithTimeout, which leaves forwarder goroutines/listeners orphaned;
change the cache to store the forwarder lifecycle (e.g., a forwarder object or
cancel/Close function) instead of just the port, return and use that forwarder
from the injection/forwarding logic, and on any failure paths (the error branch
after agentRequestWithTimeout and the other similar sites around agent
injection) call forwarder.Stop()/Close() and replace the cached forwarder
atomically when creating a new one so previous forwarders are explicitly stopped
and not leaked—update related helpers that read/write the cache to handle the
new forwarder type and ensure setCachedDeviceAgentPort usage is replaced with
setCachedDeviceAgentForwarder (or equivalent) in functions like
agentRequestWithTimeout call sites, injection/retry logic, and cleanup paths.
Comment on lines
+88
to
+108
| func (d *IOSDevice) ensureIOSDeviceAgentReady() (int, error) { | ||
| if port, ok := cachedDeviceAgentPort(d.Udid); ok && isAgentReady(port) { | ||
| utils.Verbose("reusing cached agent port %d", port) | ||
| return port, nil | ||
| } | ||
|
|
||
| if err := d.startTunnel(); err != nil { | ||
| return 0, fmt.Errorf("start tunnel: %w", err) | ||
| } | ||
|
|
||
| if port, ok := d.findRunningDeviceAgent(); ok { | ||
| setCachedDeviceAgentPort(d.Udid, port) | ||
| return port, nil | ||
| } | ||
|
|
||
| port, err := d.injectFreshAgent() | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
| setCachedDeviceAgentPort(d.Udid, port) | ||
| return port, nil |
There was a problem hiding this comment.
Serialize agent setup per device.
Only the map access is synchronized. Two concurrent agentCalls can both miss the cache and race through findRunningDeviceAgent / injectFreshAgent, which can double-attach LLDB and create competing forwards for the same UDID.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devices/ios_device_agent.go` around lines 88 - 108, Concurrent calls to
ensureIOSDeviceAgentReady can race past the cachedDeviceAgentPort check and both
call findRunningDeviceAgent/injectFreshAgent; serialize the whole setup per
device by adding a per-UDID lock and holding it for the duration of
ensureIOSDeviceAgentReady. Modify ensureIOSDeviceAgentReady to acquire a mutex
for d.Udid (e.g., add a sync.Mutex in IOSDevice or use a package-level
map[udid]*sync.Mutex) before checking cachedDeviceAgentPort and release it at
the end, ensuring the startTunnel, findRunningDeviceAgent, injectFreshAgent and
setCachedDeviceAgentPort sequence is executed under that lock so only one
goroutine performs agent setup for a given UDID.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds WebView support for physical iOS devices (
list,goto,reload,back,forward,content,eval,wait), complementing the existing simulator + Android support from #244.A real device can't
dlopenan unsigned dylib like the simulator path, so the agent is injected into the foreground app via LLDB over the go-ios CoreDevice debug proxy, then driven over a forwarded HTTP/JSON-RPC port using the shared agent protocol (agentRequest/WebViewInfo).All device-specific code lives in
devices/ios_device_webview.go(andagents/ios-real/agent.m) so it never conflicts with the shared simulator/Android webview code.Notable pieces
v1.0.211this module uses:devices/ios/debuggertools— ObjC-runtime-over-GDB calls, used to detect the foreground app without WDA.devices/ios/debugserver—GDBServerwith thebufio.Scannerbuffer raised to 64 MB. The default 64 KB cap truncated thejGetLoadedDynamicLibrariesInfosimage-list packet, which made LLDB load zero images (no symbols → expression injection failed).evaluateJavaScript's result over-released any heap value when WebKit drained its delivery pool (crashed the app oncontent). The agent now JSON-stringifies[ok, value]and copies the bytes out in C without ever retaining WebKit's object.agents/ios-real/agent.m(embedded via//go:embed) rather than a Go raw-string literal.Testing
Verified end-to-end on a real iPhone (iOS 26.5):
list,eval(strings, arrays, a 200 K string), repeatedcontent(full HTML),goto, and bounds — all succeed with the app staying alive and repeat calls hitting the reuse path.