| Message ID | 20250815102139.2200196-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás, Thank you for the patch. On Fri, Aug 15, 2025 at 12:21:39PM +0200, Barnabás Pőcze wrote: > The function is not actually thread safe contrary to its documentation. > Since it is currently not used in an unsafe context, simply remove the > mention of thread safety from the documentation. > > The variable must still remain atomic because it is accessed internally > from different threads, e.g. `Thread::postMessage()`. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/libcamera/base/thread.cpp | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp > index d8fe0d697..d7b9b2e6f 100644 > --- a/src/libcamera/base/thread.cpp > +++ b/src/libcamera/base/thread.cpp > @@ -512,8 +512,6 @@ pid_t Thread::currentId() > * This function retrieves the internal event dispatcher for the thread. The > * returned event dispatcher is valid until the thread is destroyed. > * > - * \context This function is \threadsafe. > - * Could you please also add a ASSERT(data_ == ThreadData::current()); to the implementation, to avoid future issues ? With that, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > * \return Pointer to the event dispatcher > */ > EventDispatcher *Thread::eventDispatcher()
2025. 08. 15. 12:36 keltezéssel, Laurent Pinchart írta: > Hi Barnabás, > > Thank you for the patch. > > On Fri, Aug 15, 2025 at 12:21:39PM +0200, Barnabás Pőcze wrote: >> The function is not actually thread safe contrary to its documentation. >> Since it is currently not used in an unsafe context, simply remove the >> mention of thread safety from the documentation. >> >> The variable must still remain atomic because it is accessed internally >> from different threads, e.g. `Thread::postMessage()`. >> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> src/libcamera/base/thread.cpp | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp >> index d8fe0d697..d7b9b2e6f 100644 >> --- a/src/libcamera/base/thread.cpp >> +++ b/src/libcamera/base/thread.cpp >> @@ -512,8 +512,6 @@ pid_t Thread::currentId() >> * This function retrieves the internal event dispatcher for the thread. The >> * returned event dispatcher is valid until the thread is destroyed. >> * >> - * \context This function is \threadsafe. >> - * > > Could you please also add a > > ASSERT(data_ == ThreadData::current()); > > to the implementation, to avoid future issues ? With that, > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Okay, done. I'll defer sending a new version unless other significant changes are needed. > >> * \return Pointer to the event dispatcher >> */ >> EventDispatcher *Thread::eventDispatcher() >
diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp index d8fe0d697..d7b9b2e6f 100644 --- a/src/libcamera/base/thread.cpp +++ b/src/libcamera/base/thread.cpp @@ -512,8 +512,6 @@ pid_t Thread::currentId() * This function retrieves the internal event dispatcher for the thread. The * returned event dispatcher is valid until the thread is destroyed. * - * \context This function is \threadsafe. - * * \return Pointer to the event dispatcher */ EventDispatcher *Thread::eventDispatcher()
The function is not actually thread safe contrary to its documentation. Since it is currently not used in an unsafe context, simply remove the mention of thread safety from the documentation. The variable must still remain atomic because it is accessed internally from different threads, e.g. `Thread::postMessage()`. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/libcamera/base/thread.cpp | 2 -- 1 file changed, 2 deletions(-)