Skip to content
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

Fix some potential NPEs when session is null #1784

Merged
merged 7 commits into from
Feb 15, 2022
Merged

Fix some potential NPEs when session is null #1784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2632859
Fix some potential NPEs when session is null
bilal-alsharifi Jan 7, 2022
5727e3a
Add checks for more references to session
RHenigan Feb 8, 2022
9182e91
Add more null checks and synchronized blocks
noah-livio Feb 14, 2022
73eab78
Merge branch 'bugfix/issue_1781' of origin
noah-livio Feb 14, 2022
37b307f
Merge branch 'develop' into bugfix/issue_1781
noah-livio Feb 14, 2022
3f04eba
Create and use SESSION_LOCK object
noah-livio Feb 14, 2022
85e7f29
Initialize session in synchronized block
noah-livio Feb 15, 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
102 changes: 60 additions & 42 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 @@ -76,12 +76,14 @@ public LifecycleManager(AppConfig appConfig, BaseTransportConfig config, Lifecyc
void initialize() {
super.initialize();

if (_transportConfig != null && _transportConfig.getTransportType().equals(TransportType.MULTIPLEX)) {
this.session = new SdlSession(sdlSessionListener, (MultiplexTransportConfig) _transportConfig);
} else if (_transportConfig != null && _transportConfig.getTransportType().equals(TransportType.TCP)) {
this.session = new SdlSession(sdlSessionListener, (TCPTransportConfig) _transportConfig);
} else {
DebugTool.logError(TAG, "Unable to create session for transport type");
synchronized (SESSION_LOCK) {
if (_transportConfig != null && _transportConfig.getTransportType().equals(TransportType.MULTIPLEX)) {
this.session = new SdlSession(sdlSessionListener, (MultiplexTransportConfig) _transportConfig);
} else if (_transportConfig != null && _transportConfig.getTransportType().equals(TransportType.TCP)) {
this.session = new SdlSession(sdlSessionListener, (TCPTransportConfig) _transportConfig);
} else {
DebugTool.logError(TAG, "Unable to create session for transport type");
}
}
}

Expand All @@ -93,11 +95,13 @@ void cycle(SdlDisconnectedReason 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);
}
if (session != null) {
try {
session.startSession();
} catch (SdlException e) {
e.printStackTrace();
synchronized (SESSION_LOCK) {
if (session != null) {
try {
session.startSession();
} catch (SdlException e) {
e.printStackTrace();
}
}
}
}
Expand Down Expand Up @@ -163,16 +167,18 @@ void onTransportDisconnected(String info, boolean availablePrimary, BaseTranspor
*/
@Override
void startVideoService(boolean isEncrypted, VideoStreamingParameters parameters, boolean afterPendingRestart) {
if (session == null) {
DebugTool.logWarning(TAG, "SdlSession is not created yet.");
return;
}
if (!session.getIsConnected()) {
DebugTool.logWarning(TAG, "Connection is not available.");
return;
}
synchronized (SESSION_LOCK) {
if (session == null) {
DebugTool.logWarning(TAG, "SdlSession is not created yet.");
return;
}
if (!session.getIsConnected()) {
DebugTool.logWarning(TAG, "Connection is not available.");
return;
}

session.setDesiredVideoParams(parameters);
session.setDesiredVideoParams(parameters);
}
tryStartVideoStream(isEncrypted, parameters, afterPendingRestart);
}

Expand All @@ -185,9 +191,11 @@ void startVideoService(boolean isEncrypted, VideoStreamingParameters parameters,
* @param parameters VideoStreamingParameters that are desired. Does not guarantee this is what will be accepted.
*/
private void tryStartVideoStream(boolean isEncrypted, VideoStreamingParameters parameters, boolean afterPendingRestart) {
if (session == null) {
DebugTool.logWarning(TAG, "SdlSession is not created yet.");
return;
synchronized (SESSION_LOCK) {
if (session == null) {
DebugTool.logWarning(TAG, "SdlSession is not created yet.");
return;
}
}
if (getProtocolVersion() != null && getProtocolVersion().getMajor() >= 5 && !systemCapabilityManager.isCapabilitySupported(SystemCapabilityType.VIDEO_STREAMING)) {
DebugTool.logWarning(TAG, "Module doesn't support video streaming.");
Expand All @@ -198,17 +206,20 @@ private void tryStartVideoStream(boolean isEncrypted, VideoStreamingParameters p
return;
}

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

if (afterPendingRestart || !videoServiceStartResponseReceived || !videoServiceStartResponse //If we haven't started the service before
|| (videoServiceStartResponse && isEncrypted && !session.isServiceProtected(SessionType.NAV))) { //Or the service has been started but we'd like to start an encrypted one
session.setDesiredVideoParams(parameters);

videoServiceStartResponseReceived = false;
videoServiceStartResponse = false;

addVideoServiceListener();
session.startService(SessionType.NAV, isEncrypted);

addVideoServiceListener();
if (session != null) {
session.startService(SessionType.NAV, isEncrypted);
}
}
}
}

Expand Down Expand Up @@ -237,20 +248,27 @@ public void onServiceError(SdlSession session, SessionType type, String reason)
videoServiceStartResponse = false;
}
};
session.addServiceListener(SessionType.NAV, videoServiceListener);

synchronized (SESSION_LOCK) {
if (session != null) {
session.addServiceListener(SessionType.NAV, videoServiceListener);
}
}
}
}

@Override
void startAudioService(boolean isEncrypted) {
if (session == null) {
DebugTool.logWarning(TAG, "SdlSession is not created yet.");
return;
}
if (!session.getIsConnected()) {
DebugTool.logWarning(TAG, "Connection is not available.");
return;
synchronized (SESSION_LOCK) {
if (session == null) {
DebugTool.logWarning(TAG, "SdlSession is not created yet.");
return;
}
if (!session.getIsConnected()) {
DebugTool.logWarning(TAG, "Connection is not available.");
return;
}
session.startService(SessionType.PCM, isEncrypted);
}
session.startService(SessionType.PCM, isEncrypted);
}
}
Loading