Skip to content

Fix LCM to close down properly #1796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JulianKast merged 6 commits into from
Mar 23, 2022
Merged

Fix LCM to close down properly #1796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
768f9c2
Add UAI request to clean method in LCM
joeygrover Mar 21, 2022
d00920c
Add missing initilize to JavaSE LCM
joeygrover Mar 21, 2022
d787d66
Fix incorrect start call when should stop in SdlManager
joeygrover Mar 22, 2022
fcf18d1
Add states to LCM
joeygrover Mar 22, 2022
927229f
Prevent duplicate calls to SdlManager.dispose
joeygrover Mar 22, 2022
5cde7bb
Fix formatting issues for shutdown fix
joeygrover Mar 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion android/sdl_android/src/main/java/com/smartdevicelink/managers/SdlManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,11 @@ void retryChangeRegistration() {
@SuppressLint("NewApi")
@Override
public synchronized void dispose() {
int state = getState();
if (state == BaseSubManager.SHUTDOWN || state == BaseSubManager.ERROR) {
DebugTool.logInfo(TAG, "SdlManager already disposed");
return;
}
if (this.permissionManager != null) {
this.permissionManager.dispose();
}
Expand Down Expand Up @@ -414,7 +419,7 @@ public void start() {

@Override
public void stop() {
lifecycleManager.getInternalInterface(SdlManager.this).start();
lifecycleManager.getInternalInterface(SdlManager.this).stop();
}

@Override
Expand Down
7 changes: 5 additions & 2 deletions ...id/sdl_android/src/main/java/com/smartdevicelink/managers/lifecycle/LifecycleManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@

package com.smartdevicelink.managers.lifecycle;

import static com.smartdevicelink.managers.BaseSubManager.SETTING_UP;

import android.content.Context;

import androidx.annotation.RestrictTo;
Expand Down Expand Up @@ -89,12 +91,13 @@ void initialize() {

@Override
void cycle(SdlDisconnectedReason disconnectedReason) {
clean();
initialize();
clean(true);
if (!SdlDisconnectedReason.LEGACY_BLUETOOTH_MODE_ENABLED.equals(disconnectedReason) && !SdlDisconnectedReason.PRIMARY_TRANSPORT_CYCLE_REQUEST.equals(disconnectedReason)) {
//We don't want to alert higher if we are just cycling for legacy bluetooth
onClose("Sdl Proxy Cycled", new SdlException("Sdl Proxy Cycled", SdlExceptionCause.SDL_PROXY_CYCLED), disconnectedReason);
}
transitionToState(SETTING_UP);
initialize();
synchronized (SESSION_LOCK) {
if (session != null) {
try {
Expand Down
89 changes: 53 additions & 36 deletions base/src/main/java/com/smartdevicelink/managers/lifecycle/BaseLifecycleManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@

package com.smartdevicelink.managers.lifecycle;

import static com.smartdevicelink.managers.BaseSubManager.SETTING_UP;
import static com.smartdevicelink.managers.BaseSubManager.READY;
import static com.smartdevicelink.managers.BaseSubManager.LIMITED;
import static com.smartdevicelink.managers.BaseSubManager.SHUTDOWN;
import static com.smartdevicelink.managers.BaseSubManager.ERROR;

import androidx.annotation.NonNull;
import androidx.annotation.RestrictTo;

Expand Down Expand Up @@ -117,8 +123,9 @@ abstract class BaseLifecycleManager {
ON_REQUEST_LISTENER_LOCK = new Object(),
ON_NOTIFICATION_LISTENER_LOCK = new Object();
protected static final Object SESSION_LOCK = new Object();
private final Object STATE_LOCK = new Object();


private int state;
SdlSession session;
final AppConfig appConfig;
Version rpcSpecVersion = MAX_SUPPORTED_RPC_VERSION;
Expand All @@ -143,6 +150,7 @@ abstract class BaseLifecycleManager {
DisplayCapabilities initialMediaCapabilities;

BaseLifecycleManager(AppConfig appConfig, BaseTransportConfig config, LifecycleListener listener) {
transitionToState(SETTING_UP);
this.appConfig = appConfig;
this._transportConfig = config;
this.lifecycleListener = listener;
Expand Down Expand Up @@ -175,15 +183,20 @@ public void startRPCEncryption() {
}

public synchronized void stop() {
synchronized (SESSION_LOCK) {
if (session != null) {
session.close();
session = null;
}
DebugTool.logInfo(TAG, "LifecycleManager stop requested");
clean(true);
transitionToState(SHUTDOWN);
}

protected void transitionToState(int state) {
synchronized (STATE_LOCK) {
this.state = state;
}
if (taskmaster != null) {
taskmaster.shutdown();
taskmaster = null;
}

public int getState() {
synchronized (STATE_LOCK) {
return state;
}
}

Expand Down Expand Up @@ -345,6 +358,7 @@ public OnHMIStatus getCurrentHMIStatus() {

void onClose(String info, Exception e, SdlDisconnectedReason reason) {
DebugTool.logInfo(TAG, "onClose");
transitionToState(SHUTDOWN);
if (lifecycleListener != null) {
lifecycleListener.onClosed((LifecycleManager) this, info, e, reason);
}
Expand Down Expand Up @@ -401,10 +415,7 @@ public void onReceived(RPCMessage message) {
}
if (minimumRPCVersion != null && minimumRPCVersion.isNewerThan(rpcSpecVersion) == 1) {
DebugTool.logWarning(TAG, String.format("Disconnecting from head unit, the configured minimum RPC version %s is greater than the supported RPC version %s", minimumRPCVersion, rpcSpecVersion));
UnregisterAppInterface msg = new UnregisterAppInterface();
msg.setCorrelationID(UNREGISTER_APP_INTERFACE_CORRELATION_ID);
sendRPCMessagePrivate(msg, true);
clean();
clean(true);
onClose("RPC spec version not supported: " + rpcSpecVersion.toString(), null, SdlDisconnectedReason.MINIMUM_RPC_VERSION_HIGHER_THAN_SUPPORTED);
return;
}
Expand All @@ -425,10 +436,7 @@ public void onReceived(RPCMessage message) {
boolean validSystemInfo = lifecycleListener.onSystemInfoReceived(systemInfo);
if (!validSystemInfo) {
DebugTool.logWarning(TAG, "Disconnecting from head unit, the system info was not accepted.");
UnregisterAppInterface msg = new UnregisterAppInterface();
msg.setCorrelationID(UNREGISTER_APP_INTERFACE_CORRELATION_ID);
sendRPCMessagePrivate(msg, true);
clean();
clean(true);
onClose("System not supported", null, SdlDisconnectedReason.DEFAULT);
return;
}
Expand All @@ -446,6 +454,7 @@ public void onReceived(RPCMessage message) {
DebugTool.logInfo(TAG, "on hmi status");
boolean shouldInit = currentHMIStatus == null;
currentHMIStatus = (OnHMIStatus) message;
transitionToState(READY);
if (lifecycleListener != null && shouldInit) {
lifecycleListener.onConnected((LifecycleManager) BaseLifecycleManager.this);
}
Expand Down Expand Up @@ -515,17 +524,19 @@ public void run() {

if (!onAppInterfaceUnregistered.getReason().equals(AppInterfaceUnregisteredReason.LANGUAGE_CHANGE)) {
DebugTool.logInfo(TAG, "on app interface unregistered");
clean();
clean(false);
onClose("OnAppInterfaceUnregistered received from head unit", null, SdlDisconnectedReason.APP_INTERFACE_UNREG);
} else {
DebugTool.logInfo(TAG, "re-registering for language change");
cycle(SdlDisconnectedReason.LANGUAGE_CHANGE);
}
break;
case UNREGISTER_APP_INTERFACE:
DebugTool.logInfo(TAG, "unregister app interface");
clean();
onClose("UnregisterAppInterface response received from head unit", null, SdlDisconnectedReason.APP_INTERFACE_UNREG);
DebugTool.logInfo(TAG, "Unregister app interface response received");
//Since only the library sends the UnregisterAppInterface requests, we know
//that the correct logic flows already happen based on where the call to send
//the request happens. There is also a SYNC4 bug that holds onto the response
//until the app reconnects within the same transport session.
break;
}
}
Expand Down Expand Up @@ -967,12 +978,7 @@ public void onSessionStarted(int sessionID, Version version, SystemInfo systemIn
DebugTool.logInfo(TAG, "on protocol session started");
if (minimumProtocolVersion != null && minimumProtocolVersion.isNewerThan(version) == 1) {
DebugTool.logWarning(TAG, String.format("Disconnecting from head unit, the configured minimum protocol version %s is greater than the supported protocol version %s", minimumProtocolVersion, getProtocolVersion()));
synchronized (SESSION_LOCK) {
if (session != null) {
session.endService(SessionType.RPC);
}
}
clean();
clean(false);
onClose("Protocol version not supported: " + version, null, SdlDisconnectedReason.MINIMUM_PROTOCOL_VERSION_HIGHER_THAN_SUPPORTED);
return;
}
Expand All @@ -989,14 +995,10 @@ public void onSessionStarted(int sessionID, Version version, SystemInfo systemIn
boolean validSystemInfo = lifecycleListener.onSystemInfoReceived(systemInfo);
if (!validSystemInfo) {
DebugTool.logWarning(TAG, "Disconnecting from head unit, the system info was not accepted.");
synchronized (SESSION_LOCK) {
if (session != null) {
session.endService(SessionType.RPC);
}
clean();
onClose("System not supported", null, SdlDisconnectedReason.DEFAULT);
return;
}
clean(false);
onClose("System not supported", null, SdlDisconnectedReason.DEFAULT);
return;

}
//If the vehicle is acceptable, init security lib
setSecurityLibraryIfAvailable(systemInfo.getVehicleType());
Expand Down Expand Up @@ -1280,7 +1282,20 @@ private RPCNotification handleButtonNotificationFormatting(RPCMessage notificati
return null;
}

void clean() {
void clean(boolean sendUnregisterAppInterface) {
int state = getState();
if (state == SHUTDOWN || state == ERROR) {
DebugTool.logInfo(TAG, "No need to clean, LCM is already cleaned: " + state);
return;
}

if (sendUnregisterAppInterface) {
DebugTool.logInfo(TAG, "Requesting to unregister from device");
UnregisterAppInterface uai = new UnregisterAppInterface();
uai.setCorrelationID(UNREGISTER_APP_INTERFACE_CORRELATION_ID);
sendRPCMessagePrivate(uai, true);
}

firstTimeFull = true;
currentHMIStatus = null;
lastDisplayLayoutRequestTemplate = null;
Expand All @@ -1300,6 +1315,7 @@ void clean() {
synchronized (SESSION_LOCK) {
if (session != null && session.getIsConnected()) {
session.close();
session = null;
}
}
if (encryptionLifecycleManager != null) {
Expand Down Expand Up @@ -1378,6 +1394,7 @@ void initialize() {
this.rpcRequestListeners = new HashMap<>();
this.systemCapabilityManager = new SystemCapabilityManager(internalInterface);
setupInternalRpcListeners();

}


Expand Down
8 changes: 7 additions & 1 deletion javaSE/javaSE/src/main/java/com/smartdevicelink/managers/SdlManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,12 @@ void retryChangeRegistration() {

@Override
public void dispose() {
int state = getState();
if (state == BaseSubManager.SHUTDOWN || state == BaseSubManager.ERROR) {
DebugTool.logInfo(TAG, "SdlManager already disposed");
return;
}

if (this.permissionManager != null) {
this.permissionManager.dispose();
}
Expand Down Expand Up @@ -203,7 +209,7 @@ public void start() {

@Override
public void stop() {
lifecycleManager.getInternalInterface(SdlManager.this).start();
lifecycleManager.getInternalInterface(SdlManager.this).stop();
}

@Override
Expand Down
6 changes: 5 additions & 1 deletion javaSE/javaSE/src/main/java/com/smartdevicelink/managers/lifecycle/LifecycleManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import com.smartdevicelink.session.SdlSession;
import com.smartdevicelink.transport.BaseTransportConfig;

import static com.smartdevicelink.managers.BaseSubManager.SETTING_UP;

/**
* The lifecycle manager creates a central point for all SDL session logic to converge. It should only be used by
* the library itself. Usage outside the library is not permitted and will not be protected for in the future.
Expand All @@ -57,7 +59,9 @@ void initialize() {

@Override
void cycle(SdlDisconnectedReason disconnectedReason) {
clean();
clean(true);
transitionToState(SETTING_UP);
initialize();
if (session != null) {
try {
session.startSession();
Expand Down