PostgreSQL AbortOperation - race conditions?
Posted: 10.07.2023, 11:28
This is continuation of viewtopic.php?f=10&p=245122. Short recap: if I did not missed something, variables: FLogMessage, FConn are subject to race condition when AbortOperation() is called from another thread. Even if presented analysis of source code is correct, such case will be extremely rare, and very difficult to detect, yet even harder to debug for eventual developer. There is also issue of HandleErrorOrWarning() call.
First of all, I need confirmation of drawn conclusions.
In the mean time, I searched for inspiration for possible fixes.
FConn:
PQcancel() do not mutate dereferenced content of pCancel. psycopg (Python wrapper around PostgreSQL) allocate PGCancel pointer right after FConn is created (note: I am paraphrasing, what C code of python package does in language of Zeos). We can do the same and surround PGCancel creation, potential usage and (its destruction with destruction of FConn) with mutex. FConn will not be used in AbortOperation() at all, and will not need mutex protection.
FLogMessage:
FLogMessage should be replaced with local variable.
HandleErrorOrWarning:
I do not deciphered HandleErrorOrWarning method yet, and it does a lot of things. I see 2 solutions:
1. surround it with reentrant mutex (same thread can enter it many times).
2. remove entirely, and let AbortOperation returns Integer with PG error message. <--- probably that option is unacceptable.
EDIT: Please tell me, that TZPostgreSQLConnection is not lazy-open. If it is, we have to tell the user to explicitly open db connection before any thread-related processing. Otherwise whole FConn section above looses its applicability.
First of all, I need confirmation of drawn conclusions.
In the mean time, I searched for inspiration for possible fixes.
FConn:
PQcancel() do not mutate dereferenced content of pCancel. psycopg (Python wrapper around PostgreSQL) allocate PGCancel pointer right after FConn is created (note: I am paraphrasing, what C code of python package does in language of Zeos). We can do the same and surround PGCancel creation, potential usage and (its destruction with destruction of FConn) with mutex. FConn will not be used in AbortOperation() at all, and will not need mutex protection.
FLogMessage:
FLogMessage should be replaced with local variable.
HandleErrorOrWarning:
I do not deciphered HandleErrorOrWarning method yet, and it does a lot of things. I see 2 solutions:
1. surround it with reentrant mutex (same thread can enter it many times).
2. remove entirely, and let AbortOperation returns Integer with PG error message. <--- probably that option is unacceptable.
EDIT: Please tell me, that TZPostgreSQLConnection is not lazy-open. If it is, we have to tell the user to explicitly open db connection before any thread-related processing. Otherwise whole FConn section above looses its applicability.