There is still logic error in Service

[GoogleCodeExporter]

Hi Jim,

in 2.0.17 is still an logic error in the service. The problem are the 
connecting and disconnecting boolean variables.

In the case of missing bluetooth database entry, the code simply returns from 
the receiver code, but does not reset the connecting/disconnecting booleans, so 
the service is dead afterwards, as it will no longer react on changes. This 
also leads to some ForceClose appearing later on disconnect, because the 
service "thinks" that it is still connecting while disconnecting and some state 
variables are not correctly populated.

Both receivers should at simpliest use an try...finally code like this, which 
is the correct way to prevent such races:

String results = "";
if (!connecting) try {
    connecting = true;
    recievedIntent = intent;
    if (recievedIntent == null)
        return; // <-- this may also break the receiver

    BluetoothDevice bt = (BluetoothDevice) recievedIntent
            .getExtras().get(BluetoothDevice.EXTRA_DEVICE);

    btDevice bt2 = null;
    ...
    if (bt2 == null || bt2.getMac() == null)
        return; // <-- this causes heavy problems for me
    ...
    application.sendBroadcast(itent);
} finally {
    connecting = false;
}

This try...finally code is always recommended to enforce changing variables 
before returning (in case an exception occurs or we use return like here). Java 
will then enforce that the code in finally block is executed on early exit of 
the try block.

This type of code should also be used to e.g. close resources (see also Java7's 
new try-with-resources, where this was made easier for the programmer). In 
general things like close() should be called on a finally block.

Additionally, the Java Memory Model needs the boolean variables to be declared 
"volatile", else phones with multi-core processors may not see the change, if 
two receivers are running at the same time on different cpu cores.

Volatile ensures that all updates to the connecting and disconnecting status 
variables are written through memory and so can be seen by other threads:

private volatile Intent recievedIntent = null;
private volatile boolean connecting = false;
private volatile boolean disconnecting = false;

Without volatile, this code is technically broken, which affects especially 
multi-core processors.

Original issue reported on code.google.com by uwe.h.schindler on 11 Aug 2011 at 8:53

[GoogleCodeExporter]

Original comment by JimR...@gmail.com on 11 Aug 2011 at 10:50

• Changed state: Accepted

[GoogleCodeExporter]

Original comment by JimR...@gmail.com on 11 Aug 2011 at 10:52

[GoogleCodeExporter]

Made a 2.0.18 release with bug fixes for this.  Will post here after more 
testing.

Original comment by JimR...@gmail.com on 11 Aug 2011 at 11:01

• Changed state: Started

[GoogleCodeExporter]

More service cleanup in 2.0.19 in progress.  Original issue here has been fixed 
and verified.

Original comment by JimR...@gmail.com on 12 Aug 2011 at 2:19

• Changed state: Fixed