fix(iProxy): guard releaseConnection against undefined port#2051
Merged
saikrishna321 merged 1 commit intoMay 26, 2026
Merged
Conversation
When deleteSession calls releaseConnection(udid) without a port, checkPortStatus(undefined, '127.0.0.1') threw ERR_MISSING_ARGS before the existing (!udid && !port) guard could run. The throw skipped the cleanup body, so iProxy net.Server listeners were never closed and cached connections leaked on every session end. Move the guard to the top and run the port probe / wait only when port is defined.
51ca6df to
c2104e0
Compare
d1ad9ba
into
AppiumTestDistribution:main
1 of 5 checks passed
|
Hi, @saikrishna321, could you make a release with this fix, please? |
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
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.
Bug
When a session ends,
DevicePlugin.deleteSessioncallsDEVICE_CONNECTIONS_FACTORY.releaseConnection(device.udid)(plugin.ts:757) without a port — the intent being "release ALL listeners cached under this udid". ButreleaseConnectionrunsawait checkPortStatus(port, LOCALHOST)before its(!udid && !port)guard. Withport=undefined, that resolves tonet.Socket.connect(undefined, '127.0.0.1')which throwsTypeError [ERR_MISSING_ARGS]. The throw propagates up to thecatchinplugin.ts:758, gets logged as a warn, and the cleanup body never runs.The iProxy
net.Serverinstances (WDA + MJPEG ports) are orphaned. Each iOS session leaks two ports. Over time the local port pool exhausts and subsequent sessions fail withThe port #<N> is occupied by an other processor proxy ECONNREFUSED. The only recovery is restarting Appium.A second related issue: even if the first throw is bypassed, line 278's
await this.waitForPortTobeReleased(port, isPortBusy)is also called withport=undefined, which is meaningless.Production log signature
The xcuitest line confirms its own
releaseConnectioncleans up correctly. The bug is entirely inside device-farm.Fix
Move the
(!udid && !port)guard to the top, and gate bothcheckPortStatusandwaitForPortTobeReleasedonif (port). The cleanup body (_releaseProxiedConnections,_connectionsMappingcleanup) now runs unconditionally when called with(udid, undefined)— which is the pathplugin.ts:757exercises and which I believe is the intended semantics ("release all by udid").async releaseConnection(udid: any, port?: any) { - const isPortBusy = (await checkPortStatus(port, LOCALHOST)) === 'open'; if (!udid && !port) { log.warn( 'Neither device UDID nor local port is set. ' + 'Did not know how to release the connection', ); return; } log.info(`Releasing connections for ${udid || 'any'} device on ${port || 'any'} port number`); const keys = this.listConnections(udid, port, true); // ... (unchanged) ... for (const key of keys) { delete this._connectionsMapping[key]; } - await this.waitForPortTobeReleased(port, isPortBusy); + if (port) { + const isPortBusy = (await checkPortStatus(port, LOCALHOST)) === 'open'; + await this.waitForPortTobeReleased(port, isPortBusy); + } log.debug(`Cached connections count: ${_.size(this._connectionsMapping)}`); }Reproduction
appium-device-farmplugin and one or more real iOS devices.Error while releasing connection ... ERR_MISSING_ARGSimmediately afterdeleteSession. NoiProxy ... Closing the connectionlines appear for the deleted session.The port #<N> is occupied by an other process.Validation
I compiled the patched
src/iProxy.tsstandalone withtscand verified the JS output: guard moves up, port probe wrapped inif (port), cleanup body runs unconditionally. The bug is identical onmainHEAD, so this fix applies to either base.I was unable to soak-test the fix as part of a built bundle locally —
v11.3.2references private submodules so anonymous source builds are stripped of ~280 KB of code, andmainincludes the public-ization commit (#2039) which substantively refactors the dashboard middleware in a way that breaks WDA proxying in my environment. Happy to provide more detail on the runtime log signature from production if useful.