Skip to content

Commit

Permalink
Unlock the BLE stack mutex for all callbacks to Java in AndroidDevice…
Browse files Browse the repository at this point in the history 
…ControllerWrapper. (#7290)
  • Loading branch information
@austinh0 @pull
austinh0 authored and pull[bot] committed Jun 30, 2021
1 parent 3e64cef commit 1258280
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 24 deletions.
8 changes: 6 additions & 2 deletions src/controller/java/AndroidDeviceControllerWrapper.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ JNIEnv * AndroidDeviceControllerWrapper::GetJavaEnv()
}

AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew(JavaVM * vm, jobject deviceControllerObj,
chip::NodeId nodeId, chip::System::Layer * systemLayer,
pthread_mutex_t * stackLock, chip::NodeId nodeId,
chip::System::Layer * systemLayer,
chip::Inet::InetLayer * inetLayer,
CHIP_ERROR * errInfoOnFailure)
{
Expand Down Expand Up @@ -135,7 +136,7 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew(Jav
*errInfoOnFailure = CHIP_ERROR_NO_MEMORY;
return nullptr;
}
std::unique_ptr<AndroidDeviceControllerWrapper> wrapper(new AndroidDeviceControllerWrapper(std::move(controller)));
std::unique_ptr<AndroidDeviceControllerWrapper> wrapper(new AndroidDeviceControllerWrapper(std::move(controller), stackLock));

wrapper->SetJavaObjectRef(vm, deviceControllerObj);
wrapper->Controller()->SetUdpListenPort(CHIP_PORT + 1);
Expand Down Expand Up @@ -175,16 +176,19 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew(Jav

void AndroidDeviceControllerWrapper::OnStatusUpdate(chip::Controller::DevicePairingDelegate::Status status)
{
StackUnlockGuard unlockGuard(mStackLock);
CallJavaMethod("onStatusUpdate", static_cast<jint>(status));
}

void AndroidDeviceControllerWrapper::OnPairingComplete(CHIP_ERROR error)
{
StackUnlockGuard unlockGuard(mStackLock);
CallJavaMethod("onPairingComplete", static_cast<jint>(error));
}

void AndroidDeviceControllerWrapper::OnPairingDeleted(CHIP_ERROR error)
{
StackUnlockGuard unlockGuard(mStackLock);
CallJavaMethod("onPairingDeleted", static_cast<jint>(error));
}

Expand Down
24 changes: 20 additions & 4 deletions src/controller/java/AndroidDeviceControllerWrapper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,18 @@ class AndroidDeviceControllerWrapper : public chip::Controller::DevicePairingDel
public:
~AndroidDeviceControllerWrapper();

// Use StackUnlockGuard to temporarily unlock the CHIP BLE stack, e.g. when calling application
// or Android BLE code as a result of a BLE event.
struct StackUnlockGuard
{
public:
StackUnlockGuard(pthread_mutex_t * mutex) : mMutex(mutex) { pthread_mutex_unlock(mMutex); }
~StackUnlockGuard() { pthread_mutex_lock(mMutex); }

private:
pthread_mutex_t * mMutex;
};

chip::Controller::DeviceCommissioner * Controller() { return mController.get(); }
chip::Controller::ExampleOperationalCredentialsIssuer & OpCredsIssuer() { return mOpCredsIssuer; }
void SetJavaObjectRef(JavaVM * vm, jobject obj);
Expand Down Expand Up @@ -65,24 +77,28 @@ class AndroidDeviceControllerWrapper : public chip::Controller::DevicePairingDel
return reinterpret_cast<AndroidDeviceControllerWrapper *>(handle);
}

static AndroidDeviceControllerWrapper * AllocateNew(JavaVM * vm, jobject deviceControllerObj, chip::NodeId nodeId,
chip::System::Layer * systemLayer, chip::Inet::InetLayer * inetLayer,
CHIP_ERROR * errInfoOnFailure);
static AndroidDeviceControllerWrapper * AllocateNew(JavaVM * vm, jobject deviceControllerObj, pthread_mutex_t * stackLock,
chip::NodeId nodeId, chip::System::Layer * systemLayer,
chip::Inet::InetLayer * inetLayer, CHIP_ERROR * errInfoOnFailure);

private:
using ChipDeviceControllerPtr = std::unique_ptr<chip::Controller::DeviceCommissioner>;

ChipDeviceControllerPtr mController;
chip::Controller::ExampleOperationalCredentialsIssuer mOpCredsIssuer;

pthread_mutex_t * mStackLock;

JavaVM * mJavaVM = nullptr;
jobject mJavaObjectRef = nullptr;

JNIEnv * GetJavaEnv();

jclass GetPersistentStorageClass() { return GetJavaEnv()->FindClass("chip/devicecontroller/PersistentStorage"); }

AndroidDeviceControllerWrapper(ChipDeviceControllerPtr controller) : mController(std::move(controller)) {}
AndroidDeviceControllerWrapper(ChipDeviceControllerPtr controller, pthread_mutex_t * stackLock) :
mController(std::move(controller)), mStackLock(stackLock)
{}
};

inline jlong AndroidDeviceControllerWrapper::ToJNIHandle()
Expand Down
29 changes: 11 additions & 18 deletions src/controller/java/CHIPDeviceController-JNI.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,6 @@ class ScopedPthreadLock
pthread_mutex_t * mMutex;
};

// Use StackUnlockGuard to temporarily unlock the CHIP BLE stack, e.g. when calling application
// or Android BLE code as a result of a BLE event.
struct StackUnlockGuard
{
StackUnlockGuard() { pthread_mutex_unlock(&sStackLock); }
~StackUnlockGuard() { pthread_mutex_lock(&sStackLock); }
};

} // namespace

// NOTE: Remote device ID is in sync with the echo server device id
Expand Down Expand Up @@ -209,7 +201,7 @@ jint JNI_OnLoad(JavaVM * jvm, void * reserved)
if (err != CHIP_NO_ERROR)
{
ThrowError(env, err);
StackUnlockGuard unlockGuard;
AndroidDeviceControllerWrapper::StackUnlockGuard unlockGuard(&sStackLock);
JNI_OnUnload(jvm, reserved);
}

Expand All @@ -227,7 +219,7 @@ void JNI_OnUnload(JavaVM * jvm, void * reserved)
sShutdown = true;
sSystemLayer.WakeSelect();

StackUnlockGuard unlockGuard;
AndroidDeviceControllerWrapper::StackUnlockGuard unlockGuard(&sStackLock);
pthread_join(sIOThread, NULL);
}

Expand All @@ -251,7 +243,8 @@ JNI_METHOD(jlong, newDeviceController)(JNIEnv * env, jobject self)

ChipLogProgress(Controller, "newDeviceController() called");

wrapper = AndroidDeviceControllerWrapper::AllocateNew(sJVM, self, kLocalDeviceId, &sSystemLayer, &sInetLayer, &err);
wrapper =
AndroidDeviceControllerWrapper::AllocateNew(sJVM, self, &sStackLock, kLocalDeviceId, &sSystemLayer, &sInetLayer, &err);
SuccessOrExit(err);

result = wrapper->ToJNIHandle();
Expand Down Expand Up @@ -763,7 +756,7 @@ void HandleNotifyChipConnectionClosed(BLE_CONNECTION_OBJECT connObj)
env->ExceptionClear();
tmpConnObj = reinterpret_cast<intptr_t>(connObj);
{
StackUnlockGuard unlockGuard;
AndroidDeviceControllerWrapper::StackUnlockGuard unlockGuard(&sStackLock);
env->CallStaticVoidMethod(sAndroidChipStackCls, method, static_cast<jint>(tmpConnObj));
}
VerifyOrExit(!env->ExceptionCheck(), err = CHIP_JNI_ERROR_EXCEPTION_THROWN);
Expand Down Expand Up @@ -809,7 +802,7 @@ bool HandleSendCharacteristic(BLE_CONNECTION_OBJECT connObj, const uint8_t * svc
env->ExceptionClear();
tmpConnObj = reinterpret_cast<intptr_t>(connObj);
{
StackUnlockGuard unlockGuard;
AndroidDeviceControllerWrapper::StackUnlockGuard unlockGuard(&sStackLock);
rc = (bool) env->CallStaticBooleanMethod(sAndroidChipStackCls, method, static_cast<jint>(tmpConnObj), svcIdObj, charIdObj,
characteristicDataObj);
}
Expand Down Expand Up @@ -847,7 +840,7 @@ bool HandleSubscribeCharacteristic(BLE_CONNECTION_OBJECT connObj, const uint8_t
SuccessOrExit(err);

{
StackUnlockGuard unlockGuard;
AndroidDeviceControllerWrapper::StackUnlockGuard unlockGuard(&sStackLock);
method = env->GetStaticMethodID(sAndroidChipStackCls, "onSubscribeCharacteristic", "(I[B[B)Z");
}
VerifyOrExit(method != NULL, err = CHIP_JNI_ERROR_METHOD_NOT_FOUND);
Expand Down Expand Up @@ -898,7 +891,7 @@ bool HandleUnsubscribeCharacteristic(BLE_CONNECTION_OBJECT connObj, const uint8_
env->ExceptionClear();
tmpConnObj = reinterpret_cast<intptr_t>(connObj);
{
StackUnlockGuard unlockGuard;
AndroidDeviceControllerWrapper::StackUnlockGuard unlockGuard(&sStackLock);
rc = (bool) env->CallStaticBooleanMethod(sAndroidChipStackCls, method, static_cast<jint>(tmpConnObj), svcIdObj, charIdObj);
}
VerifyOrExit(!env->ExceptionCheck(), err = CHIP_JNI_ERROR_EXCEPTION_THROWN);
Expand Down Expand Up @@ -934,7 +927,7 @@ bool HandleCloseConnection(BLE_CONNECTION_OBJECT connObj)
env->ExceptionClear();
tmpConnObj = reinterpret_cast<intptr_t>(connObj);
{
StackUnlockGuard unlockGuard;
AndroidDeviceControllerWrapper::StackUnlockGuard unlockGuard(&sStackLock);
rc = (bool) env->CallStaticBooleanMethod(sAndroidChipStackCls, method, static_cast<jint>(tmpConnObj));
}
VerifyOrExit(!env->ExceptionCheck(), err = CHIP_JNI_ERROR_EXCEPTION_THROWN);
Expand Down Expand Up @@ -962,7 +955,7 @@ uint16_t HandleGetMTU(BLE_CONNECTION_OBJECT connObj)
sJVM->GetEnv((void **) &env, JNI_VERSION_1_6);

{
StackUnlockGuard unlockGuard;
AndroidDeviceControllerWrapper::StackUnlockGuard unlockGuard(&sStackLock);
method = env->GetStaticMethodID(sAndroidChipStackCls, "onGetMTU", "(I)I");
}
VerifyOrExit(method != NULL, err = CHIP_JNI_ERROR_METHOD_NOT_FOUND);
Expand Down Expand Up @@ -1008,7 +1001,7 @@ void HandleNewConnection(void * appState, const uint16_t discriminator)

env->ExceptionClear();
{
StackUnlockGuard unlockGuard;
AndroidDeviceControllerWrapper::StackUnlockGuard unlockGuard(&sStackLock);
env->CallVoidMethod(self, method);
}
VerifyOrExit(!env->ExceptionCheck(), err = CHIP_JNI_ERROR_EXCEPTION_THROWN);
Expand Down

0 comments on commit 1258280

Please sign in to comment.