[iOS] Dispose of log readers and port forwarders if launch fails

[vashworth]

When tests run in our CI using flutter drive, if there is a failure it will loop and try again.

flutter/packages/flutter_tools/lib/src/drive/drive_service.dart

Lines 177 to 186 in 434b81f

	while (attempt < _kLaunchAttempts) {
	result = await device.startApp(
	_applicationPackage,
	mainPath: mainPath,
	route: route,
	debuggingOptions: debuggingOptions,
	platformArgs: platformArgs,
	userIdentifier: userIdentifier,
	prebuiltApplication: prebuiltApplication,
	);

However, it's using the same device instance for each iteration. So what was happening was when it would fail to launch, it would tell its listeners that it was cancelled.

flutter/packages/flutter_tools/lib/src/ios/ios_deploy.dart

Lines 486 to 489 in 434b81f

	if (_debuggerOutput.hasListener) {
	// Tell listeners the process died.
	await _debuggerOutput.close();
	}

Then when the next iteration started, the vmServiceDiscovery would immediately return with null because the deviceLogReader would be cached from the previous iteration and would already be cancelled. Therefore, bypassing and cancelling the timer.

flutter/packages/flutter_tools/lib/src/ios/devices.dart

Lines 585 to 591 in 434b81f

	localUri = await vmServiceDiscovery?.uri;
	}
	timer.cancel();
	if (localUri == null) {
	await iosDeployDebugger?.stopAndDumpBacktrace();
	return LaunchResult.failed();
	}

flutter/packages/flutter_tools/lib/src/ios/devices.dart

Line 627 in 434b81f

return _logReaders.putIfAbsent(app, () => IOSDeviceLogReader.create(

In addition, it seems like sometimes the stop would fail and therefore the the drain would never get the signal that it was done and therefore would hang forever. There was no indication that the stop had failed though because the logs were going to the stream that had no listeners since deviceLogReader was already cancelled.

flutter/packages/flutter_tools/lib/src/ios/ios_deploy.dart

Lines 563 to 576 in 434b81f

	Future<void> stopAndDumpBacktrace() async {
	if (!debuggerAttached) {
	return;
	}
	try {
	// Stop the app, which will prompt the backtrace to be printed for all threads in the stdoutSubscription handler.
	_iosDeployProcess?.stdin.writeln(_signalStop);
	} on SocketException catch (error) {
	// Best effort, try to detach, but maybe the app already exited or already detached.
	_logger.printTrace('Could not stop app from debugger: $error');
	}
	// Wait for logging to finish on process exit.
	return logLines.drain();
	}

Fixes #127141

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
• I signed the CLA.
• I listed at least one issue that this PR fixes in the description above.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[christopherfujino]

any reason not to just call dispose() from the finally block?

[vashworth]

We don't want it to dispose when it launches successfully

[christopherfujino]

ahh yeah, good point :)

[christopherfujino]

Out of curiosity, would we actually expect to see '\r' here?

[vashworth]

Oh good question, I'm not sure. I just grabbed that from the test above. I'll try and figure out

[christopherfujino]

not worth investigating, just wondering if there was a legit reason.

[vashworth]

I just happened to come across the answer to this:

flutter/packages/flutter_tools/lib/src/ios/ios_deploy.dart

Lines 477 to 480 in 4f6f188

	// The lldb console stream from ios-deploy is separated lines by an extra \r\n.
	// To avoid all lines being double spaced, if the last line from the
	// debugger was not an empty line, skip this empty line.
	// This will still cause "legit" logged newlines to be doubled...

[christopherfujino]

Is this test verifying any of your changes here? It's passing for me if I revert your other changes.

[vashworth]

Oops, let me rework it

[vashworth]

Okay updated it

[christopherfujino]

Nice! Thanks for this fix!