Fix Portal.close for java_thread_pg_entry=allow mode
#476
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Mechanically rewriting synchronized(THREADLOCK) as doInPG in c3f833b was a little too mechanical where releaseFromJava is involved. None of those DualState resource-releasing methods bother with the stack-base-fudging business, and that isn't a bug: they are not supposed to need to. DualState is already designed to enqueue those actions for later when the thread is known to be right. In allow mode, the doInPG simply fools it into performing the action immediately. Addresses issue #471.
As the ExecutionPlan reference is only held in Portal to make sure it stays live as long as the Portal does, simply move the reference into the State. That way, there is no need to bother with an extra reference to the Portal instance just to null the plan reference; that can be handled within the State itself.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
java_thread_pg_entry=allowmode is the one longest supported in PL/Java, and it ought eventually to be phased out, as its way of serializing threads' access to PG is fussy, and involves quite unconvincing manipulation of PG internal variables used for detecting excessive stack use. A better mode, not yet implemented, would be to forever allow only the original PG thread to enter PG; on entry to PL/Java, it should dispatch the Java work quickly onto some other thread and then wait as a single-thread executor service to handledoInPGrequests, until completion of the thread handling the work.However, until that better mode is implemented,
allowmode ought to be usable. Issue #471 identified a bug inPortal.closecausing it to fail depending on the thread invoking it.Although any such use of
Portal.closecould be affected, the issue was reported in Java class loading. A library that uses threads may need to enter PL/Java's classloader on an arbitrary thread; if that doesn't work, there really aren't ways to restructure the application as a workaround that aren't impractically onerous.The failure also gave unhelpful diagnostics, in which the triggering event (failure of a PG stack depth check) would be caught and unreported, leaving a later entry into PG to fail with "An attempt was made to call a PostgreSQL backend function after an
elog(ERROR)", and that would be the mysterious cause of aClassNotFoundException.This PR fixes what seems to be the only instance of the buggy coding pattern.