Data model objects can be accessed before the data model server is initialized

[kkasperczyk-no]

Problem

On nrfconnect I did observation that StartUp event is generated successfully after the boot if the device is not commissioned and the generation fails if the device was previously commissioned. The root cause of that is the event generation is scheduled by the InitChipStack and it uses data model objects even before data model server was initialized. It looks like some time race conditions may occur and sometimes data model will manage to be initialized on time and sometimes not.

Generation schedule: https://github.com/project-chip/connectedhomeip/blob/master/src/include/platform/internal/GenericPlatformManagerImpl.cpp#L126
Delegates set by data model: https://github.com/project-chip/connectedhomeip/blob/master/src/include/platform/internal/GenericPlatformManagerImpl.cpp#L297
Call of InitChipStack: https://github.com/project-chip/connectedhomeip/blob/master/examples/lock-app/nrfconnect/main/main.cpp#L46
Call of data model server init: https://github.com/project-chip/connectedhomeip/blob/master/examples/lock-app/nrfconnect/main/AppTask.cpp#L102

Problem was exposed by the cluster events behavior, but I believe it should not happen that CHIP events loop is started and processed events may try accessing data model objects, before data model server was initialized. Many platforms implement it in the similar way, so probably it affects not only nrfconnect.

Proposed Solution

• For solving cluster events issue: provide a PlatformMgr method that could be called after data model server is initialized to trigger events generation by calling HandleDeviceRebooted (https://github.com/project-chip/connectedhomeip/blob/master/src/include/platform/internal/GenericPlatformManagerImpl.cpp#L294).
• For solving general problem of data model objects accessing before server initialization: move starting event loop https://github.com/project-chip/connectedhomeip/blob/master/examples/lock-app/nrfconnect/main/main.cpp#L54 at the end of the application init code: https://github.com/project-chip/connectedhomeip/blob/master/examples/lock-app/nrfconnect/main/AppTask.cpp#L124

[bzbarsky-apple]

It looks like some time race conditions may occur

Well, there is a data race here. The init code linked to seems to be doing the following:

1. Call InitChipStack from main, which schedules HandleDeviceRebooted.
2. Call StartEventLoopTask from main, which starts a new MAtter thread and starts processing scheduled things on that thread.
3. Call StartApp from main, which calls AppTask::Init and eventually reaches the Server::Init call.

Now Server::Init is going to be seting up data model stuff on the main thread while the Matter event loop is running on the Matter thread and they are both going to be touching the same data structures at the same time.

I added asserts to catch this race in #15152 but could not figure out how to do the relevant "lock is not held" checks on Zephyr....

The second bullet of the proposed solution sounds good, in terms of ensuring that we're done touching Matter bits before the separate Matter thread starts running....

[bzbarsky-apple]

But also, the various example apps that are doing Server::Init off the Matter event loop are presumably failing to correctly handle the StartUp event, in that it consistently does not work there...

[bzbarsky-apple]

Oh, and CI is not catching this stuff because the "linux" impl we use in CI for Linux and Darwin does not use StartEventLoopTask at all: it uses the blocking RunEventLoop, so perforce does all its setup before running the event loop...

So I guess we need to enforce at minimum that Server::Init happens before the event loop starts running...

[kkasperczyk-no]

So I guess we need to enforce at minimum that Server::Init happens before the event loop starts running...

Yes, that seems to solve my issue locally. I just moved event loop start after all application initialization is over.

[bzbarsky-apple]

That said, its annoying that the way this event is generated forces all the init work to happen on the main thread, not the Matter thread....