Skip to content

Commit 0f75391

Browse files
joeygroverJulianKast
joeygrover
and
JulianKast
authored
Fix LCM to close down properly (#1796)
* Add UAI request to clean method in LCM This makes sure to send an UAI request when the LCM is shutting down to clear it from the head unit. Also removed redundent code to cose services and ignore the UAI response because there’s nothing the library can do at that point anyways. * Add missing initilize to JavaSE LCM * Fix incorrect start call when should stop in SdlManager * Add states to LCM * Prevent duplicate calls to SdlManager.dispose * Fix formatting issues for shutdown fix #1796 Co-authored-by: Julian Kast <Julian.kast@livio.io> Co-authored-by: Julian Kast <Julian.kast@livio.io>
1 parent 7094f6f commit 0f75391

File tree

5 files changed

+76
-41
lines changed

5 files changed

+76
-41
lines changed

‎android/sdl_android/src/main/java/com/smartdevicelink/managers/SdlManager.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,11 @@ void retryChangeRegistration() {
237237
@SuppressLint("NewApi")
238238
@Override
239239
public synchronized void dispose() {
240+
int state = getState();
241+
if (state == BaseSubManager.SHUTDOWN || state == BaseSubManager.ERROR) {
242+
DebugTool.logInfo(TAG, "SdlManager already disposed");
243+
return;
244+
}
240245
if (this.permissionManager != null) {
241246
this.permissionManager.dispose();
242247
}
@@ -414,7 +419,7 @@ public void start() {
414419

415420
@Override
416421
public void stop() {
417-
lifecycleManager.getInternalInterface(SdlManager.this).start();
422+
lifecycleManager.getInternalInterface(SdlManager.this).stop();
418423
}
419424

420425
@Override

‎android/sdl_android/src/main/java/com/smartdevicelink/managers/lifecycle/LifecycleManager.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232

3333
package com.smartdevicelink.managers.lifecycle;
3434

35+
import static com.smartdevicelink.managers.BaseSubManager.SETTING_UP;
36+
3537
import android.content.Context;
3638

3739
import androidx.annotation.RestrictTo;
@@ -89,12 +91,13 @@ void initialize() {
8991

9092
@Override
9193
void cycle(SdlDisconnectedReason disconnectedReason) {
92-
clean();
93-
initialize();
94+
clean(true);
9495
if (!SdlDisconnectedReason.LEGACY_BLUETOOTH_MODE_ENABLED.equals(disconnectedReason) && !SdlDisconnectedReason.PRIMARY_TRANSPORT_CYCLE_REQUEST.equals(disconnectedReason)) {
9596
//We don't want to alert higher if we are just cycling for legacy bluetooth
9697
onClose("Sdl Proxy Cycled", new SdlException("Sdl Proxy Cycled", SdlExceptionCause.SDL_PROXY_CYCLED), disconnectedReason);
9798
}
99+
transitionToState(SETTING_UP);
100+
initialize();
98101
synchronized (SESSION_LOCK) {
99102
if (session != null) {
100103
try {

‎base/src/main/java/com/smartdevicelink/managers/lifecycle/BaseLifecycleManager.java‎

Lines changed: 53 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@
3232

3333
package com.smartdevicelink.managers.lifecycle;
3434

35+
import static com.smartdevicelink.managers.BaseSubManager.SETTING_UP;
36+
import static com.smartdevicelink.managers.BaseSubManager.READY;
37+
import static com.smartdevicelink.managers.BaseSubManager.LIMITED;
38+
import static com.smartdevicelink.managers.BaseSubManager.SHUTDOWN;
39+
import static com.smartdevicelink.managers.BaseSubManager.ERROR;
40+
3541
import androidx.annotation.NonNull;
3642
import androidx.annotation.RestrictTo;
3743

@@ -117,8 +123,9 @@ abstract class BaseLifecycleManager {
117123
ON_REQUEST_LISTENER_LOCK = new Object(),
118124
ON_NOTIFICATION_LISTENER_LOCK = new Object();
119125
protected static final Object SESSION_LOCK = new Object();
126+
private final Object STATE_LOCK = new Object();
120127

121-
128+
private int state;
122129
SdlSession session;
123130
final AppConfig appConfig;
124131
Version rpcSpecVersion = MAX_SUPPORTED_RPC_VERSION;
@@ -143,6 +150,7 @@ abstract class BaseLifecycleManager {
143150
DisplayCapabilities initialMediaCapabilities;
144151

145152
BaseLifecycleManager(AppConfig appConfig, BaseTransportConfig config, LifecycleListener listener) {
153+
transitionToState(SETTING_UP);
146154
this.appConfig = appConfig;
147155
this._transportConfig = config;
148156
this.lifecycleListener = listener;
@@ -175,15 +183,20 @@ public void startRPCEncryption() {
175183
}
176184

177185
public synchronized void stop() {
178-
synchronized (SESSION_LOCK) {
179-
if (session != null) {
180-
session.close();
181-
session = null;
182-
}
186+
DebugTool.logInfo(TAG, "LifecycleManager stop requested");
187+
clean(true);
188+
transitionToState(SHUTDOWN);
189+
}
190+
191+
protected void transitionToState(int state) {
192+
synchronized (STATE_LOCK) {
193+
this.state = state;
183194
}
184-
if (taskmaster != null) {
185-
taskmaster.shutdown();
186-
taskmaster = null;
195+
}
196+
197+
public int getState() {
198+
synchronized (STATE_LOCK) {
199+
return state;
187200
}
188201
}
189202

@@ -345,6 +358,7 @@ public OnHMIStatus getCurrentHMIStatus() {
345358

346359
void onClose(String info, Exception e, SdlDisconnectedReason reason) {
347360
DebugTool.logInfo(TAG, "onClose");
361+
transitionToState(SHUTDOWN);
348362
if (lifecycleListener != null) {
349363
lifecycleListener.onClosed((LifecycleManager) this, info, e, reason);
350364
}
@@ -401,10 +415,7 @@ public void onReceived(RPCMessage message) {
401415
}
402416
if (minimumRPCVersion != null && minimumRPCVersion.isNewerThan(rpcSpecVersion) == 1) {
403417
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));
404-
UnregisterAppInterface msg = new UnregisterAppInterface();
405-
msg.setCorrelationID(UNREGISTER_APP_INTERFACE_CORRELATION_ID);
406-
sendRPCMessagePrivate(msg, true);
407-
clean();
418+
clean(true);
408419
onClose("RPC spec version not supported: " + rpcSpecVersion.toString(), null, SdlDisconnectedReason.MINIMUM_RPC_VERSION_HIGHER_THAN_SUPPORTED);
409420
return;
410421
}
@@ -425,10 +436,7 @@ public void onReceived(RPCMessage message) {
425436
boolean validSystemInfo = lifecycleListener.onSystemInfoReceived(systemInfo);
426437
if (!validSystemInfo) {
427438
DebugTool.logWarning(TAG, "Disconnecting from head unit, the system info was not accepted.");
428-
UnregisterAppInterface msg = new UnregisterAppInterface();
429-
msg.setCorrelationID(UNREGISTER_APP_INTERFACE_CORRELATION_ID);
430-
sendRPCMessagePrivate(msg, true);
431-
clean();
439+
clean(true);
432440
onClose("System not supported", null, SdlDisconnectedReason.DEFAULT);
433441
return;
434442
}
@@ -446,6 +454,7 @@ public void onReceived(RPCMessage message) {
446454
DebugTool.logInfo(TAG, "on hmi status");
447455
boolean shouldInit = currentHMIStatus == null;
448456
currentHMIStatus = (OnHMIStatus) message;
457+
transitionToState(READY);
449458
if (lifecycleListener != null && shouldInit) {
450459
lifecycleListener.onConnected((LifecycleManager) BaseLifecycleManager.this);
451460
}
@@ -515,17 +524,19 @@ public void run() {
515524

516525
if (!onAppInterfaceUnregistered.getReason().equals(AppInterfaceUnregisteredReason.LANGUAGE_CHANGE)) {
517526
DebugTool.logInfo(TAG, "on app interface unregistered");
518-
clean();
527+
clean(false);
519528
onClose("OnAppInterfaceUnregistered received from head unit", null, SdlDisconnectedReason.APP_INTERFACE_UNREG);
520529
} else {
521530
DebugTool.logInfo(TAG, "re-registering for language change");
522531
cycle(SdlDisconnectedReason.LANGUAGE_CHANGE);
523532
}
524533
break;
525534
case UNREGISTER_APP_INTERFACE:
526-
DebugTool.logInfo(TAG, "unregister app interface");
527-
clean();
528-
onClose("UnregisterAppInterface response received from head unit", null, SdlDisconnectedReason.APP_INTERFACE_UNREG);
535+
DebugTool.logInfo(TAG, "Unregister app interface response received");
536+
//Since only the library sends the UnregisterAppInterface requests, we know
537+
//that the correct logic flows already happen based on where the call to send
538+
//the request happens. There is also a SYNC4 bug that holds onto the response
539+
//until the app reconnects within the same transport session.
529540
break;
530541
}
531542
}
@@ -967,12 +978,7 @@ public void onSessionStarted(int sessionID, Version version, SystemInfo systemIn
967978
DebugTool.logInfo(TAG, "on protocol session started");
968979
if (minimumProtocolVersion != null && minimumProtocolVersion.isNewerThan(version) == 1) {
969980
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()));
970-
synchronized (SESSION_LOCK) {
971-
if (session != null) {
972-
session.endService(SessionType.RPC);
973-
}
974-
}
975-
clean();
981+
clean(false);
976982
onClose("Protocol version not supported: " + version, null, SdlDisconnectedReason.MINIMUM_PROTOCOL_VERSION_HIGHER_THAN_SUPPORTED);
977983
return;
978984
}
@@ -989,14 +995,10 @@ public void onSessionStarted(int sessionID, Version version, SystemInfo systemIn
989995
boolean validSystemInfo = lifecycleListener.onSystemInfoReceived(systemInfo);
990996
if (!validSystemInfo) {
991997
DebugTool.logWarning(TAG, "Disconnecting from head unit, the system info was not accepted.");
992-
synchronized (SESSION_LOCK) {
993-
if (session != null) {
994-
session.endService(SessionType.RPC);
995-
}
996-
clean();
997-
onClose("System not supported", null, SdlDisconnectedReason.DEFAULT);
998-
return;
999-
}
998+
clean(false);
999+
onClose("System not supported", null, SdlDisconnectedReason.DEFAULT);
1000+
return;
1001+
10001002
}
10011003
//If the vehicle is acceptable, init security lib
10021004
setSecurityLibraryIfAvailable(systemInfo.getVehicleType());
@@ -1280,7 +1282,20 @@ private RPCNotification handleButtonNotificationFormatting(RPCMessage notificati
12801282
return null;
12811283
}
12821284

1283-
void clean() {
1285+
void clean(boolean sendUnregisterAppInterface) {
1286+
int state = getState();
1287+
if (state == SHUTDOWN || state == ERROR) {
1288+
DebugTool.logInfo(TAG, "No need to clean, LCM is already cleaned: " + state);
1289+
return;
1290+
}
1291+
1292+
if (sendUnregisterAppInterface) {
1293+
DebugTool.logInfo(TAG, "Requesting to unregister from device");
1294+
UnregisterAppInterface uai = new UnregisterAppInterface();
1295+
uai.setCorrelationID(UNREGISTER_APP_INTERFACE_CORRELATION_ID);
1296+
sendRPCMessagePrivate(uai, true);
1297+
}
1298+
12841299
firstTimeFull = true;
12851300
currentHMIStatus = null;
12861301
lastDisplayLayoutRequestTemplate = null;
@@ -1300,6 +1315,7 @@ void clean() {
13001315
synchronized (SESSION_LOCK) {
13011316
if (session != null && session.getIsConnected()) {
13021317
session.close();
1318+
session = null;
13031319
}
13041320
}
13051321
if (encryptionLifecycleManager != null) {
@@ -1378,6 +1394,7 @@ void initialize() {
13781394
this.rpcRequestListeners = new HashMap<>();
13791395
this.systemCapabilityManager = new SystemCapabilityManager(internalInterface);
13801396
setupInternalRpcListeners();
1397+
13811398
}
13821399

13831400

‎javaSE/javaSE/src/main/java/com/smartdevicelink/managers/SdlManager.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,12 @@ void retryChangeRegistration() {
157157

158158
@Override
159159
public void dispose() {
160+
int state = getState();
161+
if (state == BaseSubManager.SHUTDOWN || state == BaseSubManager.ERROR) {
162+
DebugTool.logInfo(TAG, "SdlManager already disposed");
163+
return;
164+
}
165+
160166
if (this.permissionManager != null) {
161167
this.permissionManager.dispose();
162168
}
@@ -203,7 +209,7 @@ public void start() {
203209

204210
@Override
205211
public void stop() {
206-
lifecycleManager.getInternalInterface(SdlManager.this).start();
212+
lifecycleManager.getInternalInterface(SdlManager.this).stop();
207213
}
208214

209215
@Override

‎javaSE/javaSE/src/main/java/com/smartdevicelink/managers/lifecycle/LifecycleManager.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939
import com.smartdevicelink.session.SdlSession;
4040
import com.smartdevicelink.transport.BaseTransportConfig;
4141

42+
import static com.smartdevicelink.managers.BaseSubManager.SETTING_UP;
43+
4244
/**
4345
* The lifecycle manager creates a central point for all SDL session logic to converge. It should only be used by
4446
* the library itself. Usage outside the library is not permitted and will not be protected for in the future.
@@ -57,7 +59,9 @@ void initialize() {
5759

5860
@Override
5961
void cycle(SdlDisconnectedReason disconnectedReason) {
60-
clean();
62+
clean(true);
63+
transitionToState(SETTING_UP);
64+
initialize();
6165
if (session != null) {
6266
try {
6367
session.startSession();

0 commit comments

Comments
 (0)