Skip to content

Commit 9f3942a

Browse files
thsc42thsc42
committed
finally... That race condition thing seems to be solved. Runs quite smoothly actually.
1 parent e7949ec commit 9f3942a

2 files changed

Lines changed: 120 additions & 71 deletions

File tree

‎app/src/main/java/net/sharksystem/asap/android/bluetooth/BluetoothEngine.java‎

Lines changed: 23 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
import net.sharksystem.asap.android.Util;
1616
import net.sharksystem.asap.android.service2AppMessaging.ASAPServiceRequestNotifyIntent;
1717

18+
import java.io.DataInputStream;
19+
import java.io.DataOutputStream;
1820
import java.io.IOException;
1921
import java.net.Socket;
2022
import java.util.ArrayList;
@@ -358,11 +360,12 @@ private boolean checkAlreadyConnectedWithDevice(String macAddress) {
358360
if(bluetoothSocket != null) {
359361
if(bluetoothSocket.isConnected()) {
360362
Log.d(this.getLogStart(), "we already have an open and active connection to "
361-
+ macAddress);
363+
+ macAddress + " | " + bluetoothSocket);
362364
return true;
363365
} else {
364366
// there is an entry but not connected anymore (?)
365-
Log.d(this.getLogStart(), "remove socket - it is not longer (?) connected");
367+
Log.d(this.getLogStart(), "remove socket - it is not longer (?) connected "
368+
+ bluetoothSocket);
366369
this.openSockets.remove(macAddress);
367370
}
368371
}
@@ -415,6 +418,9 @@ public void checkConnectionStatus() {
415418
sb.append(connectionName);
416419
sb.append(" | isConnected: ");
417420
sb.append(bluetoothSocket.isConnected());
421+
sb.append(" | socket: ");
422+
sb.append(bluetoothSocket);
423+
418424
Log.d(this.getLogStart(), sb.toString());
419425

420426
if(!bluetoothSocket.isConnected()) {
@@ -446,7 +452,7 @@ void handleBTSocket(BluetoothSocket socket, boolean isClient) throws IOException
446452
String remoteMacAddress = socket.getRemoteDevice().getAddress();
447453

448454
String logMessage = isClient ? "Client" : "Server";
449-
logMessage += "socket called: handle new BT connection to ";
455+
logMessage += "socket called: handle new BT connection" + socket;
450456

451457
Log.d(this.getLogStart(), logMessage + remoteMacAddress);
452458

@@ -456,80 +462,26 @@ void handleBTSocket(BluetoothSocket socket, boolean isClient) throws IOException
456462
return;
457463
}
458464

459-
/* There is a race condition which is not that obvious:
460-
Bot phones A and B are going to initiate a connection. Both also offer a server socket.
461-
Assumed, A and B initiate a connection in the same moment (create a client socket).
462-
Both would get a connection and launch an ASAP session. If the timing is bad - and that's
463-
more likely as I wished - both would be asked from their server sockets to handle a
464-
new connection as well.
465-
466-
In principle, that is what we want. With a bad timing that would happen on both sides, though.
467-
In that case, both would realize that there is already an existing communication channel
468-
(their own TCP client socket) and close the server side socket. Again, that is what we
469-
want - but not on both ends.
470-
471-
Both connections would be killed. We want one TCP channel to be closed. But only one.
472-
473-
Solution: We implement a bias. Let's say. A smaller remote mac address should use a
474-
client socket more likely.
475-
*/
476-
477-
long myNumber = this.makeANumberFromMyMacAddress(this.mBluetoothAdapter.getAddress());
478-
long remoteNumber = this.makeANumberFromMyMacAddress(remoteMacAddress);
479-
480-
try {
481-
if(remoteNumber < myNumber) {
482-
if(isClient) {
483-
Log.d(this.getLogStart(), "block my client socket a moment: " + socket);
484-
Thread.sleep(500); // let local client socket wait some time
485-
Log.d(this.getLogStart(), "my client socket back in the race: " + socket);
486-
}
465+
// avoid the nasty race condition
466+
boolean waited = this.waitBeforeASAPSessionLaunch(
467+
socket.getInputStream(),
468+
socket.getOutputStream(),
469+
isClient, 500);
470+
471+
// ask again?
472+
if(waited) {
473+
if (this.checkAlreadyConnectedWithDevice(remoteMacAddress)) {
474+
socket.close();
475+
return;
487476
}
488-
else
489-
if(!isClient) {
490-
Log.d(this.getLogStart(), "block my server socket a moment: "+ socket);
491-
Thread.sleep(500); // opposite
492-
Log.d(this.getLogStart(), "my server socket back in the race: " + socket);
493-
}
494-
} catch (InterruptedException e) {
495-
e.printStackTrace();
496-
}
497-
498-
this.handleBTSocket(socket);
499-
}
500-
501-
private long makeANumberFromMyMacAddress(String macAddress) {
502-
String[] split = macAddress.split(":");
503-
504-
long value = 0;
505-
for(String part : split) {
506-
try {
507-
value += Long.parseLong(part);
508-
}
509-
catch (NumberFormatException e) {
510-
// ignore - we just need a number
511-
}
512-
}
513-
514-
return value;
515-
}
516-
517-
private synchronized void handleBTSocket(BluetoothSocket socket) throws IOException {
518-
Log.d(this.getLogStart(), "in synchronized handleSocket()");
519-
String address = socket.getRemoteDevice().getAddress();
520-
521-
// already an open connection?
522-
if (this.checkAlreadyConnectedWithDevice(address)) {
523-
socket.close();
524-
return;
525477
}
526478

527479
// remember that new connection
528-
Log.d(this.getLogStart(), "remember socket");
529-
this.openSockets.put(address, socket);
480+
Log.d(this.getLogStart(), "remember socket: " + socket);
481+
this.openSockets.put(remoteMacAddress, socket);
530482

531483
Log.d(this.getLogStart(), "launch asap session");
532-
this.launchASAPConnection(address, socket.getInputStream(), socket.getOutputStream());
484+
this.launchASAPConnection(remoteMacAddress, socket.getInputStream(), socket.getOutputStream());
533485
}
534486

535487
public void propagateStatus(Context ctx) throws ASAPException {

‎app/src/main/java/net/sharksystem/asap/android/service/MacLayerEngine.java‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,22 @@
11
package net.sharksystem.asap.android.service;
22

33
import android.content.Context;
4+
import android.net.wifi.WifiInfo;
5+
import android.net.wifi.WifiManager;
46
import android.util.Log;
57

68
import net.sharksystem.asap.ASAPException;
79
import net.sharksystem.asap.protocol.ASAPConnection;
810

11+
import java.io.DataInputStream;
12+
import java.io.DataOutputStream;
913
import java.io.IOException;
1014
import java.io.InputStream;
1115
import java.io.OutputStream;
1216
import java.util.Date;
1317
import java.util.HashMap;
1418
import java.util.Map;
19+
import java.util.Random;
1520
import java.util.UUID;
1621

1722
public abstract class MacLayerEngine {
@@ -25,6 +30,7 @@ public abstract class MacLayerEngine {
2530
// public static final long DEFAULT_WAIT_BEFORE_RECONNECT_TIME = 1000*60; // a minute
2631
public static final long DEFAULT_WAIT_BEFORE_RECONNECT_TIME = 1000; // a second - debugging
2732
private final long waitBeforeReconnect;
33+
private final int randomValue; // to avoid a nasty race condition
2834

2935
public MacLayerEngine(ASAPService asapService, Context context) {
3036
this(asapService, context, DEFAULT_WAIT_BEFORE_RECONNECT_TIME);
@@ -34,6 +40,9 @@ public MacLayerEngine(ASAPService asapService, Context context, long waitingPeri
3440
this.asapService = asapService;
3541
this.context = context;
3642
this.waitBeforeReconnect = waitingPeriod;
43+
44+
Random random = new Random(System.currentTimeMillis());
45+
this.randomValue = random.nextInt();
3746
}
3847

3948
protected Context getContext() {
@@ -111,6 +120,19 @@ private String getLogStart() {
111120

112121
private Map<String, ASAPConnection> asapConnections = new HashMap<>();
113122

123+
private String localMacAddress = null;
124+
public String getLocalMacAddress() {
125+
if(localMacAddress == null) {
126+
WifiManager wifiManager = (WifiManager)
127+
asapService.getApplicationContext().getSystemService(Context.WIFI_SERVICE);
128+
129+
WifiInfo wInfo = wifiManager.getConnectionInfo();
130+
this.localMacAddress = wInfo.getMacAddress();
131+
}
132+
133+
return this.localMacAddress;
134+
}
135+
114136
/**
115137
* kill connection to address
116138
* @param address
@@ -142,5 +164,80 @@ protected void launchASAPConnection(
142164
}
143165
}
144166

167+
/** There is a race condition which is not that obvious: Example Bluetooth
168+
Bot phones A and B are going to initiate a connection. Both also offer a server socket.
169+
Assumed, A and B initiate a connection in the same moment (create a client socket).
170+
Both would get a connection and launch an ASAP session. If the timing is bad - and that's
171+
more likely as I wished - both would be asked from their server sockets to handle a
172+
new connection as well.
173+
174+
In principle, that is what we want. With a bad timing that would happen on both sides, though.
175+
In that case, both would realize that there is already an existing communication channel
176+
(their own TCP client socket) and close the server side socket. Again, that is what we
177+
want - but not on both ends.
178+
179+
Both connections would be killed. We want one TCP channel to be closed. But only one.
180+
181+
Solution: We implement a bias: This method is called with an additional parameter (initiator).
182+
In TCP, we could call it client socket. Connection was initiated by creating a TCP port. It can be used
183+
in any way but it must be ensured: Both side will not use same boolean value
184+
*/
185+
public boolean waitBeforeASAPSessionLaunch(InputStream is, OutputStream os,
186+
boolean connectionInitiator, long waitInMillis) throws IOException {
187+
// run a little negotiation before we start
188+
DataOutputStream dos = new DataOutputStream(os);
189+
int remoteValue = 0;
190+
191+
try {
192+
dos.writeInt(this.randomValue);
193+
DataInputStream dis = new DataInputStream(is);
194+
remoteValue = dis.readInt();
195+
} catch (IOException e) {
196+
// decision is made - this connection is gone anyway
197+
os.close();
198+
is.close();
199+
}
200+
201+
StringBuilder sb = new StringBuilder();
202+
sb.append("try to solve race condition: localValue == ");
203+
sb.append(this.randomValue);
204+
sb.append(" | remoteValue == ");
205+
sb.append(remoteValue);
206+
sb.append(" | initiator == ");
207+
sb.append(connectionInitiator);
208+
209+
int initiatorValue, nonInitiatorValue;
210+
if(connectionInitiator) {
211+
initiatorValue = this.randomValue;
212+
nonInitiatorValue = remoteValue;
213+
} else {
214+
initiatorValue = remoteValue;
215+
nonInitiatorValue = this.randomValue;
216+
}
217+
218+
sb.append(" | initiatorValue == ");
219+
sb.append(initiatorValue);
220+
sb.append(" | nonInitiatorValue == ");
221+
sb.append(nonInitiatorValue);
222+
Log.d(this.getLogStart(), sb.toString());
223+
224+
/* Here comes the bias: An initiator with a smaller value waits a moment */
225+
if(connectionInitiator & initiatorValue < nonInitiatorValue) {
226+
try {
227+
sb = new StringBuilder();
228+
sb.append("wait ");
229+
sb.append(waitInMillis);
230+
sb.append(" ms");
231+
Log.d(this.getLogStart(), sb.toString());
232+
Thread.sleep(waitInMillis);
233+
return true;
234+
} catch (InterruptedException e) {
235+
Log.d(this.getLogStart(), "wait interrupted");
236+
}
237+
}
238+
239+
return false;
240+
}
241+
145242
public abstract void checkConnectionStatus();
146243
}

0 commit comments

Comments
 (0)