Skip to content

Commit

Permalink
[#24237] YSQL: Fix data race in regex pushdown
Browse files Browse the repository at this point in the history
Summary:
D36095 / d0500a0 addressed a correctness issue in regular expression pushdown: `regexp.c` was written for single-threaded Postgres, and so has static variables to maintain state. With expression pushdown enabled, the tserver will run `regexp.c` in multiple threads. To solve this, the static variables were changed to thread local variables.

#24237 uncovered a similar issue, this time with static variables in `regc_pg_locale.c`. The same approach was used as before - define the variables as thread local and use the thread local variables if we're in the multi threaded environment.

There are 4 static variables in `regc_pg_locale.c`:
```
static PG_Locale_Strategy pg_regex_strategy;
static pg_locale_t pg_regex_locale;
static Oid	pg_regex_collation;
static pg_ctype_cache *pg_ctype_cache_list = NULL;
```

Because the final one has `malloc` allocations, moving it to a thread-local variable will cause memory leaks unless the deallocation is moved to the C++ lifecycle. That makes it a more complex change, so I'll address that as part of #25561.
Jira: DB-13132

Test Plan: ./yb_build.sh tsan --cxx-test pgwrapper_pg_mini-test --gtest_filter PgMiniTest.RegexPushdown

Reviewers: amartsinchyk, dmitry

Reviewed By: amartsinchyk

Subscribers: jason, yql

Differential Revision: https://phorge.dev.yugabyte.com/D41133
  • Loading branch information
timothy-e committed Jan 15, 2025
1 parent 1a9aef4 commit 2b4598f
Show file tree
Hide file tree
Showing 6 changed files with 143 additions and 54 deletions.
Loading

0 comments on commit 2b4598f

Please sign in to comment.