Skip to content

Commit 9182e91

Browse files
noah-livionoah-livio
committed
Add more null checks and synchronized blocks
1 parent 2632859 commit 9182e91

2 files changed

Lines changed: 134 additions & 78 deletions

File tree

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

Lines changed: 52 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -93,11 +93,13 @@ void cycle(SdlDisconnectedReason disconnectedReason) {
9393
//We don't want to alert higher if we are just cycling for legacy bluetooth
9494
onClose("Sdl Proxy Cycled", new SdlException("Sdl Proxy Cycled", SdlExceptionCause.SDL_PROXY_CYCLED), disconnectedReason);
9595
}
96-
if (session != null) {
97-
try {
98-
session.startSession();
99-
} catch (SdlException e) {
100-
e.printStackTrace();
96+
synchronized (session) {
97+
if (session != null) {
98+
try {
99+
session.startSession();
100+
} catch (SdlException e) {
101+
e.printStackTrace();
102+
}
101103
}
102104
}
103105
}
@@ -163,16 +165,18 @@ void onTransportDisconnected(String info, boolean availablePrimary, BaseTranspor
163165
*/
164166
@Override
165167
void startVideoService(boolean isEncrypted, VideoStreamingParameters parameters, boolean afterPendingRestart) {
166-
if (session == null) {
167-
DebugTool.logWarning(TAG, "SdlSession is not created yet.");
168-
return;
169-
}
170-
if (!session.getIsConnected()) {
171-
DebugTool.logWarning(TAG, "Connection is not available.");
172-
return;
173-
}
168+
synchronized (session) {
169+
if (session == null) {
170+
DebugTool.logWarning(TAG, "SdlSession is not created yet.");
171+
return;
172+
}
173+
if (!session.getIsConnected()) {
174+
DebugTool.logWarning(TAG, "Connection is not available.");
175+
return;
176+
}
174177

175-
session.setDesiredVideoParams(parameters);
178+
session.setDesiredVideoParams(parameters);
179+
}
176180
tryStartVideoStream(isEncrypted, parameters, afterPendingRestart);
177181
}
178182

@@ -185,9 +189,11 @@ void startVideoService(boolean isEncrypted, VideoStreamingParameters parameters,
185189
* @param parameters VideoStreamingParameters that are desired. Does not guarantee this is what will be accepted.
186190
*/
187191
private void tryStartVideoStream(boolean isEncrypted, VideoStreamingParameters parameters, boolean afterPendingRestart) {
188-
if (session == null) {
189-
DebugTool.logWarning(TAG, "SdlSession is not created yet.");
190-
return;
192+
synchronized (session) {
193+
if (session == null) {
194+
DebugTool.logWarning(TAG, "SdlSession is not created yet.");
195+
return;
196+
}
191197
}
192198
if (getProtocolVersion() != null && getProtocolVersion().getMajor() >= 5 && !systemCapabilityManager.isCapabilitySupported(SystemCapabilityType.VIDEO_STREAMING)) {
193199
DebugTool.logWarning(TAG, "Module doesn't support video streaming.");
@@ -198,17 +204,20 @@ private void tryStartVideoStream(boolean isEncrypted, VideoStreamingParameters p
198204
return;
199205
}
200206

207+
synchronized (session) {
208+
if (afterPendingRestart || !videoServiceStartResponseReceived || !videoServiceStartResponse //If we haven't started the service before
209+
|| (videoServiceStartResponse && isEncrypted && !session.isServiceProtected(SessionType.NAV))) { //Or the service has been started but we'd like to start an encrypted one
210+
if (session != null) {
211+
session.setDesiredVideoParams(parameters);
212+
}
213+
videoServiceStartResponseReceived = false;
214+
videoServiceStartResponse = false;
201215

202-
if (afterPendingRestart || !videoServiceStartResponseReceived || !videoServiceStartResponse //If we haven't started the service before
203-
|| (videoServiceStartResponse && isEncrypted && !session.isServiceProtected(SessionType.NAV))) { //Or the service has been started but we'd like to start an encrypted one
204-
session.setDesiredVideoParams(parameters);
205-
206-
videoServiceStartResponseReceived = false;
207-
videoServiceStartResponse = false;
208-
209-
addVideoServiceListener();
210-
session.startService(SessionType.NAV, isEncrypted);
211-
216+
addVideoServiceListener();
217+
if (session != null) {
218+
session.startService(SessionType.NAV, isEncrypted);
219+
}
220+
}
212221
}
213222
}
214223

@@ -237,20 +246,27 @@ public void onServiceError(SdlSession session, SessionType type, String reason)
237246
videoServiceStartResponse = false;
238247
}
239248
};
240-
session.addServiceListener(SessionType.NAV, videoServiceListener);
249+
250+
synchronized (session) {
251+
if (session != null) {
252+
session.addServiceListener(SessionType.NAV, videoServiceListener);
253+
}
254+
}
241255
}
242256
}
243257

244258
@Override
245259
void startAudioService(boolean isEncrypted) {
246-
if (session == null) {
247-
DebugTool.logWarning(TAG, "SdlSession is not created yet.");
248-
return;
249-
}
250-
if (!session.getIsConnected()) {
251-
DebugTool.logWarning(TAG, "Connection is not available.");
252-
return;
260+
synchronized (session) {
261+
if (session == null) {
262+
DebugTool.logWarning(TAG, "SdlSession is not created yet.");
263+
return;
264+
}
265+
if (!session.getIsConnected()) {
266+
DebugTool.logWarning(TAG, "Connection is not available.");
267+
return;
268+
}
269+
session.startService(SessionType.PCM, isEncrypted);
253270
}
254-
session.startService(SessionType.PCM, isEncrypted);
255271
}
256272
}

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

Lines changed: 82 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,10 @@ abstract class BaseLifecycleManager {
151151

152152
public void start() {
153153
try {
154-
if (session != null) {
155-
session.startSession();
154+
synchronized (session) {
155+
if (session != null) {
156+
session.startSession();
157+
}
156158
}
157159
} catch (SdlException e) {
158160
DebugTool.logError(TAG,"Error attempting to start session", e);
@@ -163,15 +165,19 @@ public void start() {
163165
* Start a secured RPC service
164166
*/
165167
public void startRPCEncryption() {
166-
if (session != null) {
167-
session.startService(SessionType.RPC, true);
168+
synchronized (session) {
169+
if (session != null) {
170+
session.startService(SessionType.RPC, true);
171+
}
168172
}
169173
}
170174

171175
public synchronized void stop() {
172-
if (session != null) {
173-
session.close();
174-
session = null;
176+
synchronized (session) {
177+
if (session != null) {
178+
session.close();
179+
session = null;
180+
}
175181
}
176182
if (taskmaster != null) {
177183
taskmaster.shutdown();
@@ -196,8 +202,10 @@ Taskmaster getTaskmaster() {
196202
}
197203

198204
Version getProtocolVersion() {
199-
if (session != null && session.getProtocolVersion() != null) {
200-
return session.getProtocolVersion();
205+
synchronized (session) {
206+
if (session != null && session.getProtocolVersion() != null) {
207+
return session.getProtocolVersion();
208+
}
201209
}
202210
return new Version(1, 0, 0);
203211
}
@@ -303,10 +311,12 @@ public SystemCapabilityManager getSystemCapabilityManager(SdlManager sdlManager)
303311
}
304312

305313
private boolean isConnected() {
306-
if (session != null) {
307-
return session.getIsConnected();
308-
} else {
309-
return false;
314+
synchronized (session) {
315+
if (session != null) {
316+
return session.getIsConnected();
317+
} else {
318+
return false;
319+
}
310320
}
311321
}
312322

@@ -401,7 +411,11 @@ public void onReceived(RPCMessage message) {
401411
VehicleType vehicleType = raiResponse.getVehicleType();
402412
String systemSoftwareVersion = raiResponse.getSystemSoftwareVersion();
403413
if (vehicleType != null || systemSoftwareVersion != null) {
404-
saveVehicleType(session.getActiveTransports(), vehicleType);
414+
synchronized (session) {
415+
if (session != null) {
416+
saveVehicleType(session.getActiveTransports(), vehicleType);
417+
}
418+
}
405419
SystemInfo systemInfo = new SystemInfo(vehicleType, systemSoftwareVersion, null);
406420
boolean validSystemInfo = lifecycleListener.onSystemInfoReceived(systemInfo);
407421
if (!validSystemInfo) {
@@ -801,8 +815,10 @@ private void sendRPCMessagePrivate(RPCMessage message, boolean isInternalMessage
801815

802816
final ProtocolMessage pm = new ProtocolMessage();
803817
pm.setData(msgBytes);
804-
if (session != null) {
805-
pm.setSessionID((byte) session.getSessionId());
818+
synchronized (session) {
819+
if (session != null) {
820+
pm.setSessionID((byte) session.getSessionId());
821+
}
806822
}
807823

808824
pm.setMessageType(MessageType.RPC);
@@ -870,8 +886,10 @@ private void sendRPCMessagePrivate(RPCMessage message, boolean isInternalMessage
870886
pm.setPriorityCoefficient(1);
871887
}
872888

873-
if (session != null) {
874-
session.sendMessage(pm);
889+
synchronized (session) {
890+
if (session != null) {
891+
session.sendMessage(pm);
892+
}
875893
}
876894

877895
} catch (OutOfMemoryError e) {
@@ -944,22 +962,30 @@ public void onSessionStarted(int sessionID, Version version, SystemInfo systemIn
944962
DebugTool.logInfo(TAG, "on protocol session started");
945963
if (minimumProtocolVersion != null && minimumProtocolVersion.isNewerThan(version) == 1) {
946964
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()));
947-
session.endService(SessionType.RPC);
965+
synchronized (session) {
966+
if (session != null) {
967+
session.endService(SessionType.RPC);
968+
}
969+
}
948970
clean();
949971
onClose("Protocol version not supported: " + version, null, SdlDisconnectedReason.MINIMUM_PROTOCOL_VERSION_HIGHER_THAN_SUPPORTED);
950972
return;
951973
}
952974

953975
if (systemInfo != null && lifecycleListener != null) {
954976
didCheckSystemInfo = true;
955-
saveVehicleType(session.getActiveTransports(), systemInfo.getVehicleType());
956-
boolean validSystemInfo = lifecycleListener.onSystemInfoReceived(systemInfo);
957-
if (!validSystemInfo) {
958-
DebugTool.logWarning(TAG, "Disconnecting from head unit, the system info was not accepted.");
959-
session.endService(SessionType.RPC);
960-
clean();
961-
onClose("System not supported", null, SdlDisconnectedReason.DEFAULT);
962-
return;
977+
synchronized (session) {
978+
if (session != null) {
979+
saveVehicleType(session.getActiveTransports(), systemInfo.getVehicleType());
980+
boolean validSystemInfo = lifecycleListener.onSystemInfoReceived(systemInfo);
981+
if (!validSystemInfo) {
982+
DebugTool.logWarning(TAG, "Disconnecting from head unit, the system info was not accepted.");
983+
session.endService(SessionType.RPC);
984+
clean();
985+
onClose("System not supported", null, SdlDisconnectedReason.DEFAULT);
986+
return;
987+
}
988+
}
963989
}
964990
//If the vehicle is acceptable, init security lib
965991
setSecurityLibraryIfAvailable(systemInfo.getVehicleType());
@@ -1028,8 +1054,10 @@ public void stop() {
10281054
@Override
10291055
public boolean isConnected() {
10301056
synchronized (BaseLifecycleManager.this) {
1031-
if (BaseLifecycleManager.this.session != null) {
1032-
return BaseLifecycleManager.this.session.getIsConnected();
1057+
synchronized (BaseLifecycleManager.this.session) {
1058+
if (BaseLifecycleManager.this.session != null) {
1059+
return BaseLifecycleManager.this.session.getIsConnected();
1060+
}
10331061
}
10341062
}
10351063
return false;
@@ -1038,17 +1066,21 @@ public boolean isConnected() {
10381066
@Override
10391067
public void addServiceListener(SessionType serviceType, ISdlServiceListener sdlServiceListener) {
10401068
synchronized (BaseLifecycleManager.this) {
1041-
if(BaseLifecycleManager.this.session != null ){
1042-
BaseLifecycleManager.this.session.addServiceListener(serviceType, sdlServiceListener);
1069+
synchronized (BaseLifecycleManager.this.session) {
1070+
if (BaseLifecycleManager.this.session != null) {
1071+
BaseLifecycleManager.this.session.addServiceListener(serviceType, sdlServiceListener);
1072+
}
10431073
}
10441074
}
10451075
}
10461076

10471077
@Override
10481078
public void removeServiceListener(SessionType serviceType, ISdlServiceListener sdlServiceListener) {
10491079
synchronized (BaseLifecycleManager.this) {
1050-
if (BaseLifecycleManager.this.session != null) {
1051-
BaseLifecycleManager.this.session.removeServiceListener(serviceType, sdlServiceListener);
1080+
synchronized (BaseLifecycleManager.this.session) {
1081+
if (BaseLifecycleManager.this.session != null) {
1082+
BaseLifecycleManager.this.session.removeServiceListener(serviceType, sdlServiceListener);
1083+
}
10521084
}
10531085
}
10541086
}
@@ -1118,8 +1150,10 @@ public RegisterAppInterfaceResponse getRegisterAppInterfaceResponse() {
11181150
@Override
11191151
public boolean isTransportForServiceAvailable(SessionType serviceType) {
11201152
synchronized (BaseLifecycleManager.this) {
1121-
if (BaseLifecycleManager.this.session != null) {
1122-
return BaseLifecycleManager.this.session.isTransportForServiceAvailable(serviceType);
1153+
synchronized (BaseLifecycleManager.this.session) {
1154+
if (BaseLifecycleManager.this.session != null) {
1155+
return BaseLifecycleManager.this.session.isTransportForServiceAvailable(serviceType);
1156+
}
11231157
}
11241158
}
11251159
return false;
@@ -1148,8 +1182,10 @@ public Version getProtocolVersion() {
11481182
@Override
11491183
public long getMtu(SessionType serviceType) {
11501184
synchronized (BaseLifecycleManager.this) {
1151-
if (BaseLifecycleManager.this.session != null) {
1152-
return BaseLifecycleManager.this.session.getMtu(serviceType);
1185+
synchronized (BaseLifecycleManager.this.session) {
1186+
if (BaseLifecycleManager.this.session != null) {
1187+
return BaseLifecycleManager.this.session.getMtu(serviceType);
1188+
}
11531189
}
11541190
}
11551191
return SdlProtocolBase.V1_V2_MTU_SIZE;
@@ -1250,8 +1286,10 @@ void clean() {
12501286
if (rpcRequestListeners != null) {
12511287
rpcRequestListeners.clear();
12521288
}
1253-
if (session != null && session.getIsConnected()) {
1254-
session.close();
1289+
synchronized (session) {
1290+
if (session != null && session.getIsConnected()) {
1291+
session.close();
1292+
}
12551293
}
12561294
if (encryptionLifecycleManager != null) {
12571295
encryptionLifecycleManager.dispose();
@@ -1306,9 +1344,11 @@ private void setSecurityLibraryIfAvailable(VehicleType vehicleType) {
13061344
if ((sec != null) && (sec.getMakeList() != null)) {
13071345
if (sec.getMakeList().contains(make)) {
13081346
sec.setAppId(appConfig.getAppID());
1309-
if (session != null) {
1310-
session.setSdlSecurity(sec);
1311-
sec.handleSdlSession(session);
1347+
synchronized (session) {
1348+
if (session != null) {
1349+
session.setSdlSecurity(sec);
1350+
sec.handleSdlSession(session);
1351+
}
13121352
}
13131353
return;
13141354
}

0 commit comments

Comments
 (0)