Thread safety?

[sfackler]

Is this library intended to be thread safe? I seem to be able to be able to segfault it when running these tests in parallel, but not when limiting it to a single thread: https://github.com/sfackler/rust-pg_query/blob/2169d608e0f4df4f17a7798f662393f9dd3f9e66/pg_query/src/lib.rs#L68

pg_query ❯ env RUST_TEST_THREADS=1 cargo test
     Running target/debug/pg_query-773a16cd18fc4144

running 2 tests
test test::err ... ok
test test::ok ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured

   Doc-tests pg_query

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured

pg_query ❯ env RUST_TEST_THREADS=2 cargo test
     Running target/debug/pg_query-773a16cd18fc4144

running 2 tests
WARNING:  01000: problem in alloc set pg_query_parse: detected write past chunk end in block 0x7f9568219000, chunk 0x7f9568219190
LOCATION:  AllocSetCheck, aset.c:1354
WARNING:  01000: problem in alloc set pg_query_parse: bad single-chunk 0x7f95682191b0 in block 0x7f9568219000
LOCATION:  AllocSetCheck, aset.c:1337
WARNING:  01000: problem in alloc set pg_query_parse: bogus aset link in block 0x7f9568219000, chunk 0x7f95682191b0
LOCATION:  AllocSetCheck, aset.c:1346
WARNING:  01000: problem in alloc set pg_query_parse: detected write past chunk end in block 0x7f9568219000, chunk 0x7f95682191b0
LOCATION:  AllocSetCheck, aset.c:1354
WARNING:  01000: problem in alloc set pg_query_parse: found inconsistent memory block 0x7f9568219000
LOCATION:  AllocSetCheck, aset.c:1364
error: Process didn't exit successfully: `/home/sfackler/rust/rust-pg_query/pg_query/target/debug/pg_query-773a16cd18fc4144` (signal: 11, SIGSEGV: invalid memory reference)

To learn more, run the command again with --verbose.'

[lfittl]

@sfackler Apologies, there should be a big warning about this, but I didn't add one - its not thread-safe right now. In concurrent languages you need a mutex around the parser (thats how I've done the Go binding).

I'm curious, was it previously thread safe for you when linking to the full PostgreSQL source? (I haven't investigated if the memory management and parser can be safely used in a threaded context)

[sfackler]

Cool, makes sense.

I'm not sure about the thread safety story with the old setup - it was only ever used in a single threaded context.

[lfittl]

@sfackler Makes sense. Do you think it'd be helpful if the C library itself had a mutex around the parser? (i.e. instead of adding them in all the other languages)

[lfittl]

@sfackler Alright, so I've debugged this a bit more. We might actually be able to make it thread-safe by using thread-local storage for all global variables.

Even just marking the memory management variables __thread seems to do the trick. I've pushed up a test branch in https://github.com/lfittl/libpg_query/tree/thread-safety - maybe you can give it a try?

(still need to work this transformation into the LLVM extraction code, but that should be fairly straightforward)