Skip to content

Commit 7873fb5

Browse files
Move router service message sending to its own thread (#1871)
* Added queueing to send messages to router service via transport broker on a different thread * update unit test * Remove old logic * Add new class to hold all task queue logic for the TransportBroker * Refactor logic in TransportBroker to new class created * fix comment * Rename SendMessageToRouterService * Refactor SendToRouterServiceTaskMaster into base * Update thread name for RouterServiceMessageEmitter * Remove extra threadin queue object for RS emiiter * Refctor emitter callback * Remove extra emitter task class * Merge adding to tail in emitter * Update method modifiers in emitter * Remove extra method in emitter for polling * Fix increment to retryCount in transportBroker * Remove custom linked list in favor of LinkedList in the emitter. * Add null checks in emitter * Remove throwing NPE in emitter * Additional cleanup for #1871 * Add library scope annotation to emitter --------- Co-authored-by: “JKAST” <julian.kast@live.com>
1 parent 30fe848 commit 7873fb5

3 files changed

Lines changed: 125 additions & 11 deletions

File tree

android/sdl_android/src/androidTest/java/com/smartdevicelink/transport/TransportBrokerTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,24 +117,24 @@ public void testSendMessageToRouterService() {
117117
broker.routerServiceMessenger = null;
118118
broker.isBound = true;
119119

120-
assertFalse(broker.sendMessageToRouterService(message));
120+
assertFalse(broker.sendMessageToRouterService(message, 0));
121121

122122
broker.routerServiceMessenger = new Messenger(handler); //So it's not ambiguous
123123

124124
broker.isBound = false;
125125

126-
assertFalse(broker.sendMessageToRouterService(message));
126+
assertFalse(broker.sendMessageToRouterService(message, 0));
127127

128128
broker.isBound = true;
129129
broker.registeredWithRouterService = true;
130130

131131
message = null;
132132

133-
assertFalse(broker.sendMessageToRouterService(message));
133+
assertFalse(broker.sendMessageToRouterService(message, 0));
134134

135135
message = new Message();
136136

137-
assertTrue(broker.sendMessageToRouterService(message));
137+
assertTrue(broker.sendMessageToRouterService(message, 0));
138138

139139
}
140140

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
package com.smartdevicelink.transport;
2+
3+
import android.os.Message;
4+
5+
import androidx.annotation.RestrictTo;
6+
7+
import java.util.LinkedList;
8+
import java.util.Queue;
9+
10+
@RestrictTo(RestrictTo.Scope.LIBRARY)
11+
public class RouterServiceMessageEmitter extends Thread {
12+
13+
protected final Object QUEUE_LOCK = new Object();
14+
private boolean isHalted = false, isWaiting = false;
15+
private final Callback callback;
16+
private final Queue<Message> queue = new LinkedList<>();
17+
18+
public RouterServiceMessageEmitter(Callback callback) {
19+
this.setName("RouterServiceMessageEmitter");
20+
this.setDaemon(true);
21+
this.callback = callback;
22+
}
23+
24+
@Override
25+
public void run() {
26+
while (!isHalted) {
27+
try {
28+
Message message;
29+
synchronized (QUEUE_LOCK) {
30+
message = getNextTask();
31+
if (message != null && callback != null) {
32+
callback.onMessageToSend(message);
33+
} else {
34+
isWaiting = true;
35+
QUEUE_LOCK.wait();
36+
isWaiting = false;
37+
}
38+
}
39+
} catch (InterruptedException e) {
40+
break;
41+
}
42+
}
43+
}
44+
45+
protected void alert() {
46+
if (isWaiting) {
47+
synchronized (QUEUE_LOCK) {
48+
QUEUE_LOCK.notify();
49+
}
50+
}
51+
}
52+
53+
protected void close() {
54+
this.isHalted = true;
55+
if (queue != null) {
56+
queue.clear();
57+
}
58+
}
59+
60+
/**
61+
* Insert the task in the queue where it belongs
62+
*
63+
* @param message the new Message that needs to be added to the queue to be handled
64+
*/
65+
public void add(Message message) {
66+
synchronized (this) {
67+
if (message != null && queue != null) {
68+
queue.add(message);
69+
}
70+
}
71+
}
72+
73+
/**
74+
* Remove the head of the queue
75+
*
76+
* @return the old head of the queue
77+
*/
78+
private Message getNextTask() {
79+
synchronized (this) {
80+
if (queue != null) {
81+
return queue.poll();
82+
} else {
83+
return null;
84+
}
85+
}
86+
}
87+
88+
protected interface Callback {
89+
boolean onMessageToSend(Message message);
90+
}
91+
}

android/sdl_android/src/main/java/com/smartdevicelink/transport/TransportBroker.java

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ public class TransportBroker {
9191

9292
Messenger routerServiceMessenger = null;
9393
final Messenger clientMessenger;
94+
private RouterServiceMessageEmitter routerServiceMessageEmitter;
9495

9596
boolean isBound = false, registeredWithRouterService = false;
9697
private String routerPackage = null, routerClassName = null;
@@ -109,6 +110,13 @@ private void initRouterConnection() {
109110

110111
public void onServiceConnected(ComponentName className, IBinder service) {
111112
DebugTool.logInfo(TAG, "Bound to service " + className.toString());
113+
routerServiceMessageEmitter = new RouterServiceMessageEmitter(new RouterServiceMessageEmitter.Callback() {
114+
@Override
115+
public boolean onMessageToSend(Message message) {
116+
return sendMessageToRouterService(message, 0);
117+
}
118+
});
119+
routerServiceMessageEmitter.start();
112120
routerServiceMessenger = new Messenger(service);
113121
isBound = true;
114122
//So we just established our connection
@@ -118,7 +126,7 @@ public void onServiceConnected(ComponentName className, IBinder service) {
118126

119127
public void onServiceDisconnected(ComponentName className) {
120128
DebugTool.logInfo(TAG, "Unbound from service " + className.getClassName());
121-
routerServiceMessenger = null;
129+
shutDownRouterServiceMessenger();
122130
registeredWithRouterService = false;
123131
isBound = false;
124132
onHardwareDisconnected(null, null);
@@ -127,7 +135,12 @@ public void onServiceDisconnected(ComponentName className) {
127135
}
128136

129137
protected boolean sendMessageToRouterService(Message message) {
130-
return sendMessageToRouterService(message, 0);
138+
if (routerServiceMessageEmitter != null) {
139+
routerServiceMessageEmitter.add(message);
140+
routerServiceMessageEmitter.alert();
141+
}
142+
// Updated to only return true as we have added sending messages to SdlRouterService to be on a different thread.
143+
return true;
131144
}
132145

133146
protected boolean sendMessageToRouterService(Message message, int retryCount) {
@@ -152,7 +165,7 @@ protected boolean sendMessageToRouterService(Message message, int retryCount) {
152165
} catch (InterruptedException e1) {
153166
e1.printStackTrace();
154167
}
155-
return sendMessageToRouterService(message, retryCount++);
168+
return sendMessageToRouterService(message, ++retryCount);
156169
} else {
157170
//DeadObject, time to kill our connection
158171
DebugTool.logInfo(TAG, "Dead object while attempting to send packet");
@@ -431,7 +444,7 @@ public boolean start() {
431444
public void resetSession() {
432445
synchronized (INIT_LOCK) {
433446
unregisterWithRouterService();
434-
routerServiceMessenger = null;
447+
shutDownRouterServiceMessenger();
435448
unBindFromRouterService();
436449
isBound = false;
437450
}
@@ -445,7 +458,7 @@ public void stop() {
445458
synchronized (INIT_LOCK) {
446459
unregisterWithRouterService();
447460
unBindFromRouterService();
448-
routerServiceMessenger = null;
461+
shutDownRouterServiceMessenger();
449462
currentContext = null;
450463

451464
}
@@ -629,8 +642,7 @@ private void unregisterWithRouterService() {
629642
} else {
630643
DebugTool.logWarning(TAG, "Unable to unregister, not bound to router service");
631644
}
632-
633-
routerServiceMessenger = null;
645+
shutDownRouterServiceMessenger();
634646
}
635647

636648
protected ComponentName getRouterService() {
@@ -747,4 +759,15 @@ public void removeSession(long sessionId) {
747759
msg.setData(bundle);
748760
this.sendMessageToRouterService(msg);
749761
}
762+
763+
/**
764+
* Method to shut down RouterServiceMessenger
765+
*/
766+
private void shutDownRouterServiceMessenger() {
767+
routerServiceMessenger = null;
768+
if (routerServiceMessageEmitter != null) {
769+
routerServiceMessageEmitter.close();
770+
}
771+
routerServiceMessageEmitter = null;
772+
}
750773
}

0 commit comments

Comments
 (0)