Skip to content

Commit

Permalink
Add a custom switch to clean up the leaked Dart VM (flutter#30541)
Browse files Browse the repository at this point in the history
  • Loading branch information
eggfly authored Jan 21, 2022
1 parent 79a961d commit 0ba963f
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 0 deletions.
4 changes: 4 additions & 0 deletions common/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,10 @@ struct Settings {
// shells in the platform (via their embedding APIs) should cooperate to make
// sure this flag is never set if they want the VM to shutdown and free all
// associated resources.
// It can be customized by application, more detail:
// https://github.com/flutter/flutter/issues/95903
// TODO(eggfly): Should it be set to false by default?
// https://github.com/flutter/flutter/issues/96843
bool leak_vm = true;

// Engine settings
Expand Down
4 changes: 4 additions & 0 deletions shell/common/switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,10 @@ Settings SettingsFromCommandLine(const fml::CommandLine& command_line) {
command_line.GetOptionValue(FlagForSwitch(Switch::CacheDirPath),
&settings.temp_directory_path);

bool leak_vm = "true" == command_line.GetOptionValueWithDefault(
FlagForSwitch(Switch::LeakVM), "true");
settings.leak_vm = leak_vm;

if (settings.icu_initialization_required) {
command_line.GetOptionValue(FlagForSwitch(Switch::ICUDataFilePath),
&settings.icu_data_path);
Expand Down
5 changes: 5 additions & 0 deletions shell/common/switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,11 @@ DEF_SWITCH(EnableSkParagraph,
"enable-skparagraph",
"Selects the SkParagraph implementation of the text layout engine.")

DEF_SWITCH(LeakVM,
"leak-vm",
"When the last shell shuts down, the shared VM is leaked by default "
"(the leak_vm in VM settings is true). To clean up the leak VM, set "
"this value to false.")
DEF_SWITCHES_END

void PrintUsage(const std::string& executable_name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,19 @@ public class FlutterLoader {
private static final String ENABLE_SKPARAGRAPH_META_DATA_KEY =
"io.flutter.embedding.android.EnableSkParagraph";

/**
* Set whether leave or clean up the VM after the last shell shuts down. It can be set from app's
* meta-data in <application /> in AndroidManifest.xml. Set it to true in to leave the Dart VM,
* set it to false to destroy VM.
*
* <p>If your want to let your app destroy the last shell and re-create shells more quickly, set
* it to true, otherwise if you want to clean up the memory of the leak VM, set it to false.
*
* <p>TODO(eggfly): Should it be set to false by default?
* https://github.com/flutter/flutter/issues/96843
*/
private static final String LEAK_VM_META_DATA_KEY = "io.flutter.embedding.android.LeakVM";

// Must match values in flutter::switches
static final String AOT_SHARED_LIBRARY_NAME = "aot-shared-library-name";
static final String AOT_VMSERVICE_SHARED_LIBRARY_NAME = "aot-vmservice-shared-library-name";
Expand Down Expand Up @@ -299,6 +312,9 @@ public void ensureInitializationComplete(
shellArgs.add("--enable-skparagraph");
}

final String leakVM = isLeakVM(metaData) ? "true" : "false";
shellArgs.add("--leak-vm=" + leakVM);

long initTimeMillis = SystemClock.uptimeMillis() - initStartTimestampMillis;

flutterJNI.init(
Expand All @@ -318,6 +334,14 @@ public void ensureInitializationComplete(
}
}

private static boolean isLeakVM(@Nullable Bundle metaData) {
final boolean leakVMDefaultValue = true;
if (metaData == null) {
return leakVMDefaultValue;
}
return metaData.getBoolean(LEAK_VM_META_DATA_KEY, leakVMDefaultValue);
}

/**
* Same as {@link #ensureInitializationComplete(Context, String[])} but waiting on a background
* thread, then invoking {@code callback} on the {@code callbackHandler}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import android.annotation.TargetApi;
import android.app.ActivityManager;
import android.content.Context;
import android.os.Bundle;
import io.flutter.embedding.engine.FlutterJNI;
import java.util.Arrays;
import java.util.List;
Expand Down Expand Up @@ -85,6 +86,58 @@ public void itDefaultsTheOldGenHeapSizeAppropriately() {
assertTrue(arguments.contains(oldGenHeapArg));
}

@Test
public void itSetsLeakVMToTrueByDefault() {
FlutterJNI mockFlutterJNI = mock(FlutterJNI.class);
FlutterLoader flutterLoader = new FlutterLoader(mockFlutterJNI);

assertFalse(flutterLoader.initialized());
flutterLoader.startInitialization(RuntimeEnvironment.application);
flutterLoader.ensureInitializationComplete(RuntimeEnvironment.application, null);
shadowOf(getMainLooper()).idle();

final String leakVMArg = "--leak-vm=true";
ArgumentCaptor<String[]> shellArgsCaptor = ArgumentCaptor.forClass(String[].class);
verify(mockFlutterJNI, times(1))
.init(
eq(RuntimeEnvironment.application),
shellArgsCaptor.capture(),
anyString(),
anyString(),
anyString(),
anyLong());
List<String> arguments = Arrays.asList(shellArgsCaptor.getValue());
assertTrue(arguments.contains(leakVMArg));
}

@Test
public void itSetsTheLeakVMFromMetaData() {
FlutterJNI mockFlutterJNI = mock(FlutterJNI.class);
FlutterLoader flutterLoader = new FlutterLoader(mockFlutterJNI);
Bundle metaData = new Bundle();
metaData.putBoolean("io.flutter.embedding.android.LeakVM", false);
RuntimeEnvironment.application.getApplicationInfo().metaData = metaData;

FlutterLoader.Settings settings = new FlutterLoader.Settings();
assertFalse(flutterLoader.initialized());
flutterLoader.startInitialization(RuntimeEnvironment.application, settings);
flutterLoader.ensureInitializationComplete(RuntimeEnvironment.application, null);
shadowOf(getMainLooper()).idle();

final String leakVMArg = "--leak-vm=false";
ArgumentCaptor<String[]> shellArgsCaptor = ArgumentCaptor.forClass(String[].class);
verify(mockFlutterJNI, times(1))
.init(
eq(RuntimeEnvironment.application),
shellArgsCaptor.capture(),
anyString(),
anyString(),
anyString(),
anyLong());
List<String> arguments = Arrays.asList(shellArgsCaptor.getValue());
assertTrue(arguments.contains(leakVMArg));
}

@Test
public void itUsesCorrectExecutorService() {
FlutterJNI mockFlutterJNI = mock(FlutterJNI.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,13 @@
NSNumber* enableSkParagraph = [mainBundle objectForInfoDictionaryKey:@"FLTEnableSkParagraph"];
settings.enable_skparagraph = (enableSkParagraph != nil) ? enableSkParagraph.boolValue : false;

// Leak Dart VM settings, set whether leave or clean up the VM after the last shell shuts down.
NSNumber* leakDartVM = [mainBundle objectForInfoDictionaryKey:@"FLTLeakDartVM"];
// It will change the default leak_vm value in settings only if the key exists.
if (leakDartVM != nil) {
settings.leak_vm = leakDartVM.boolValue;
}

#if FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG
// There are no ownership concerns here as all mappings are owned by the
// embedder and not the engine.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@ - (void)testMainBundleSettingsAreCorrectlyParsed {
[FlutterDartProject domainNetworkPolicy:appTransportSecurity]);
}

- (void)testLeakDartVMSettingsAreCorrectlyParsed {
// The FLTLeakDartVM's value is defined in Info.plist
NSBundle* mainBundle = [NSBundle mainBundle];
NSNumber* leakDartVM = [mainBundle objectForInfoDictionaryKey:@"FLTLeakDartVM"];
XCTAssertEqual(leakDartVM.boolValue, NO);

auto settings = FLTDefaultSettingsForBundle();
// Check settings.leak_vm value is same as the value defined in Info.plist.
XCTAssertEqual(settings.leak_vm, NO);
}

- (void)testEmptySettingsAreCorrect {
XCTAssertFalse([FlutterDartProject allowsArbitraryLoads:[[NSDictionary alloc] init]]);
XCTAssertEqualObjects(@"", [FlutterDartProject domainNetworkPolicy:[[NSDictionary alloc] init]]);
Expand Down
2 changes: 2 additions & 0 deletions testing/ios/IosUnitTests/App/Info.plist
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
</dict>
</dict>
</dict>
<key>FLTLeakDartVM</key>
<false/>
<key>UIRequiredDeviceCapabilities</key>
<array>
<string>armv7</string>
Expand Down

0 comments on commit 0ba963f

Please sign in to comment.