Re: [PATCH 11/11] android/tester: Make bt_callbacks thread-safe

[Marcel Holtmann <marcel@holtmann.org>]

Hi Anderson,

>> This patch adds wrappers for BT HAL callback which execute them in main
>> thread instead of notification_handler() thread. Otherwise test
>> execution is prone to race conditions since we do not provide any
>> locking mechanism for tester.
> 
> In my opinion, this is becoming too messy. I'm getting races even
> inside the emulator code: sometimes bthost->ncmd becomes zero before a
> HCI command is sent by the emulated host because the Command Status /
> Command Complete comes after the command is written to the socket, but
> before bthost->ncmd is decremented.
> 
> Also try running android-tester under valgrind. At least for me, I get
> a few failures that I don't have when running without valgrind (at
> least one in HIDHost apparently due to if_bluetooth->enable() not
> being called on test setup and thus the tests rely on finishing before
> the controller is powered off by the kernel after initialization).
> 
> IMHO, the best approach would be to keep all HAL API usage in a
> separate process, and keep android-tester single-threaded. Of course,
> this could extra complexity for the required IPC between
> android-tester and this new process...
> 
> Again, I'm not familiar with how HAL API works, all this is just based
> on my failed attempt to make android-tester run reliably under
> valgrind.

I have to agree. We might better spawn processes for this. Our emulator code was never designed to be thread safe and never will be. We are just hiding the real problem here and it will break somewhere else later on.

Regards

Marcel