Handle C bindings exit

[latot]

Hi! I has been using pgrx, the experience in general has been nice!

In order use it, and extend functionalities I has been using C bindings with other postgres extension, I has been easily to connect PGRX + C Bindings! I have already tested this.

There is one issue I have found using them, is when a C bindings calls exit function, this one makes PG crash, and PGRX crash (for example using pgrx tests).

At first I thought to handle this situation inside each extension, but any binding from any extension can call exit, so maybe is better handle this situation from PGRX, if it can handle the exit would be a lot easier to write extensions that needs bindings.

I think bindings are nice here, helps to extend actual extensions and slowly move parts from C to Rust.
This is not only because I'm making bindings from a postgres extension, any binding from any C app that uses exit would have this behaviour.

I think I can try write a basic case for tests, I think we can talk about this if think is needed I can upload one here.

Thx!

[usamoi]

There is one issue I have found using them, is when a C bindings calls exit function, this one makes PG crash

exit kills the process, isn't it expected? Could you explain why exit is called?

[latot]

Hi! this depends on the extension or C app we are binding.

I checked some things, at least in PG extensions can be usual, for example I played with postgis extension, if someone sent a bad text input of a geometry will exit because is a unrecoverable error.

In PG we have a lot of circumstances when we want to stop it from continue that can leads to exit there is also ereport which should also stop it internally (a guess).

C does not have a good way to handle this types of errors, as you know Result is not there, so any error of that type can ends with a exit(1) for example.

Also other C apps can also have that behaviour, each app is free to call it...

How exit stop the process, if we handle this inside each extensions would be very complex, each call should be done in a new process, handle shared memory for pointers and across several platforms.

If PGRX catch this, it only needs to stop right, and make PG stop, and nothing more would be need for the extensions to handle this.

What I don't know, is how this would mix with for example, PG shared variables, no idea how they mix.

[latot]

Note: obvs, bindings to apps that are not a pg extension, can not use ereport which can lead to a exit call.

[usamoi]

I still don't understand, why not use ereport(ERROR)?

[latot]

Some times a design decision, other times, does not belongs to the app, not all C apps are designed to work on postgres to use it, lets not reduce this scenario to only pg extension bindings :)

[usamoi]

Do you mean that you're using some C libraries that don't belong to any PostgreSQL extensions, and their only way of handling errors is calling exit, right?

[usamoi]

I believe the correct approach in this case is to patch your C library so that it calls ereport. If you neither override the exit symbol at link time nor use LD_PRELOAD to forcibly override it, then there is no reliable way at runtime to prevent exit from terminating the entire process.

In principle, a C library shouldn't use exit to handle errors. At least not for input-related errors. It should return an error code instead.

[latot]

The main scenario is bindings to C apps that uses exit(1) or with different error codes.

mm, patch C does not seems reliable.

To do it, we would need to patch not only the C app, all its dependencies, rebuild from scratch everything to make it works.
And not all cases is easy to replace with ereport, also need to change all the build packages....!!
Some cases just include the postgres.h changes the behaviour of other functions...

Touch C apps to make it works with PG does not sounds an option we can actually do if want to use bindings.
We can't really force to all ppl of C code work in a particular way... there can be better ways to write a lot of code, I agree with that.

I have never tried to override a symbol, and it should be done in a instance that only affects the current library and its dependencies.

Is possible to make pgrx run a new process, and keep the main one to handle this scenario?

[usamoi]

Is possible to make pgrx run a new process, and keep the main one to handle this scenario?

pgrx_tests can't handle this. If you only want to run tests (and don't care about crashes in real use of the extension), you can use other testing tools such as sqllogictest.

Then you can write a few lines of shell script to start sqllogictest for each test, so that each test runs in a separate process, like this:

cargo pgrx start pg17
for file in $(find ./tests -type f); do
    sqllogictest "$file" -d NAME_OF_YOUR_EXTENSION -u $USER
done
cargo pgrx stop pg17

[latot]

Some tests are also in rust and uses the C bindings, I have run some tests with psql, but how the pg backed process crash it stops :(

Without missing the point, we do not really want to do an extra thing on exit(1), actually PG allow extensions use it!
But when is called from bindings makes all crash, including the PG session.

[usamoi]

actually PG allow extensions use it

Yes, and the expected result of using it is aborting the process. It's how exit works. This is determined by its POSIX semantics, and we can do nothing about it.

If you want exit not to terminate the process, that's something we cannot make.

[latot]

I want it to terminate!

This is the scenario:

If exit is called directly from C, it works fine.
If exit is called from a C binding, crashed the whole session in the server (similar if we call the tests).

So, the idea is just to terminate right.