Re: [PATCH v2] libselinux: improve performance with pcre matches

On Fri, Jul 24, 2026 at 12:44 AM Stephen Smalley
<[email protected]> wrote:
>
> On Thu, Jul 23, 2026 at 4:58 AM Inseob Kim <[email protected]> wrote:
> >
> > From: Carlo Marcelo Arenas Belón <[email protected]>
> >
> > Since 30b3e9d2 (libselinux: Workaround for heap overhead of pcre,
> > 2023-01-12), performance of PCRE2 matches has been affected due to
> > excesive recreation of the match_data in an attempt to reduce memory
>
> spelling /excessive/
>
> > utilization; instead of a workaround, it would be better to address
> > the problem and maybe even improve performance in the process.
> >
> > The issue is that currently the structure that holds PCRE state has
> > both a pcre2_code (which is per pattern) and a pcre2_match_data (which
> > is per match), forcing us to add a mutex to prevent multiple matches to
> > step on each other.
> >
> > Lets remove the match_data and the mutex and instead allocate one once
> > in a thread independent way that could be used and reused, by extending
> > our pthread interface to not only store TLS variables but also retrieve
> > them, and then use one of those.
> >
> > Since we are not interested on the capture groups (if any) lets only
> > allocate 1 pair which is all that will be needed and change the logic
> > so that a return of 0 (which means the pattern matched but there were
> > not enough capture spots) is also considered a match.
> >
> > This will ensure that the memory use would be bound to the number of
> > concurrent matches instead of the number of patterns and therefore
> > reduce the impact that recent changes on the way that the frames used
> > for matching are allocated might had brough since 10.41 was released.
>
> should this be "have brought"? And is 10.41 the pcre version?
>
> >
> > For cases where threads are not available, just keep it working in slow
> > mode as done before the workaround was reverted.
> >
> > Signed-off-by: Carlo Marcelo Arenas Belón <[email protected]>
> > Signed-off-by: Inseob Kim <[email protected]>
> > ---
> > v2:
> >  - Fix the dual destructor antipattern by separating the thread-exit
> >    destructor (match_data_thread_free) from the library-unload key cleanup
> >    (match_data_key_destroy, annotated with __attribute__((destructor))).
>
> This won't free the data on dlclose() of libselinux, right? Not sure if we care.

You're correct, it will leak on dlclose(). And I believe we don't
care. Freeing that memory would require maintaining a global list of
active threads, which is infeasible. And dynamically unloading
libselinux is extremely rare anyway.

>
> >  - Eliminate the data race on match_data_key_initialized by using pthread_once
> >    (via __selinux_once) for safe, race-free initialization of match_data_key.
> >  - Remove redundant match_data_initialized thread-local variable, making
> >    lookups purely based on __selinux_getspecific().
> > ---