Re: Making plpython 2 and 3 coexist a bit better - Mailing list pgsql-hackers

Jim Nasby <Jim.Nasby@BlueTreble.com> writes:
> Something that's always concerned me about functions in other languages
> is that any kind of snafu in the function/language can hose the backend,
> which you may or may not detect. I've used other databases that (by
> default) spin up a separate process for executing functions, maybe we
> could do something like that? If we treated 2 and 3 as different
> languages you could actually use both at the same time in a single
> backend. The only thing that's not clear to me is how you'd be able to
> re-enter the process during recursive/nested calls.

There's at least one PL/Java implementation that does that.  The
interprocess communication overhead is pretty awful, IIRC.  Don't know
what they do about nested calls.

> Obviously this is a lot more work than what you're proposing though. :(

Yeah.  I think what I'm suggesting is a back-patchable fix, which that
certainly wouldn't be.

I realized that the patch I put up before would do the Wrong Thing if
an updated library were loaded followed by a not-updated library for
the other python version.  Ideally that couldn't happen, but considering
that the two plpythons are likely to get packaged in separate subpackages
(certainly Red Hat does things that way), it's not too hard to envision
cases where it would.  So the attached update corrects that and fixes the
docs too.  We can lose the whole test in HEAD, but I think it's necessary
in the back branches.

The question of whether to do ERROR or FATAL remains open.  I'm not sure
I have a strong preference either way.

            regards, tom lane

diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
index 015bbad..03f6bcc 100644
*** a/doc/src/sgml/plpython.sgml
--- b/doc/src/sgml/plpython.sgml
***************
*** 176,183 ****
     based on Python 3 in the same session, because the symbols in the
     dynamic modules would clash, which could result in crashes of the
     PostgreSQL server process.  There is a check that prevents mixing
!    Python major versions in a session, which will abort the session if
!    a mismatch is detected.  It is possible, however, to use both
     PL/Python variants in the same database, from separate sessions.
    </para>
   </sect1>
--- 176,183 ----
     based on Python 3 in the same session, because the symbols in the
     dynamic modules would clash, which could result in crashes of the
     PostgreSQL server process.  There is a check that prevents mixing
!    Python major versions within a session.
!    It is possible, however, to use both
     PL/Python variants in the same database, from separate sessions.
    </para>
   </sect1>
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index 3c2ebfa..6805148 100644
*** a/src/pl/plpython/plpy_main.c
--- b/src/pl/plpython/plpy_main.c
*************** static void PLy_init_interp(void);
*** 63,68 ****
--- 63,71 ----
  static PLyExecutionContext *PLy_push_execution_context(void);
  static void PLy_pop_execution_context(void);

+ /* static state for Python library conflict detection */
+ static int *plpython_version_bitmask_ptr = NULL;
+ static int    plpython_version_bitmask = 0;
  static const int plpython_python_version = PY_MAJOR_VERSION;

  /* initialize global variables */
*************** static PLyExecutionContext *PLy_executio
*** 75,102 ****
  void
  _PG_init(void)
  {
!     /* Be sure we do initialization only once (should be redundant now) */
!     static bool inited = false;
      const int **version_ptr;

!     if (inited)
!         return;

!     /* Be sure we don't run Python 2 and 3 in the same session (might crash) */
      version_ptr = (const int **) find_rendezvous_variable("plpython_python_version");
      if (!(*version_ptr))
          *version_ptr = &plpython_python_version;
      else
      {
!         if (**version_ptr != plpython_python_version)
              ereport(FATAL,
                      (errmsg("Python major version mismatch in session"),
                       errdetail("This session has previously used Python major version %d, and it is now attempting to
usePython major version %d.", 
                                 **version_ptr, plpython_python_version),
                       errhint("Start a new session to use a different Python major version.")));
      }

!     pg_bindtextdomain(TEXTDOMAIN);

  #if PY_MAJOR_VERSION >= 3
      PyImport_AppendInittab("plpy", PyInit_plpy);
--- 78,155 ----
  void
  _PG_init(void)
  {
!     int          **bitmask_ptr;
      const int **version_ptr;

!     /*
!      * Set up a shared bitmask variable telling which Python version(s) are
!      * loaded into this process's address space.  If there's more than one, we
!      * cannot call into libpython for fear of causing crashes.  But postpone
!      * the actual failure for later, so that operations like pg_restore can
!      * load more than one plpython library so long as they don't try to do
!      * anything much with the language.
!      */
!     bitmask_ptr = (int **) find_rendezvous_variable("plpython_version_bitmask");
!     if (!(*bitmask_ptr))        /* am I the first? */
!         *bitmask_ptr = &plpython_version_bitmask;
!     /* Retain pointer to the agreed-on shared variable ... */
!     plpython_version_bitmask_ptr = *bitmask_ptr;
!     /* ... and announce my presence */
!     *plpython_version_bitmask_ptr |= (1 << PY_MAJOR_VERSION);

!     /*
!      * This should be safe even in the presence of conflicting plpythons, and
!      * it's necessary to do it here for the next error to be localized.
!      */
!     pg_bindtextdomain(TEXTDOMAIN);
!
!     /*
!      * We used to have a scheme whereby PL/Python would fail immediately if
!      * loaded into a session in which a conflicting libpython is already
!      * present.  We don't like to do that anymore, but it seems possible that
!      * a plpython library adhering to the old convention is present in the
!      * session, in which case we have to fail.  We detect an old library if
!      * plpython_python_version is already defined but the indicated version
!      * isn't reflected in plpython_version_bitmask.  Otherwise, set the
!      * variable so that the right thing happens if an old library is loaded
!      * later.
!      */
      version_ptr = (const int **) find_rendezvous_variable("plpython_python_version");
      if (!(*version_ptr))
          *version_ptr = &plpython_python_version;
      else
      {
!         if ((*plpython_version_bitmask_ptr & (1 << **version_ptr)) == 0)
              ereport(FATAL,
                      (errmsg("Python major version mismatch in session"),
                       errdetail("This session has previously used Python major version %d, and it is now attempting to
usePython major version %d.", 
                                 **version_ptr, plpython_python_version),
                       errhint("Start a new session to use a different Python major version.")));
      }
+ }

! /*
!  * Perform one-time setup of PL/Python, after checking for a conflict
!  * with other versions of Python.
!  */
! static void
! PLy_initialize(void)
! {
!     static bool inited = false;
!
!     /*
!      * Check for multiple Python libraries before actively doing anything with
!      * libpython.  This must be repeated on each entry to PL/Python, in case a
!      * conflicting library got loaded since we last looked.
!      */
!     if (*plpython_version_bitmask_ptr != (1 << PY_MAJOR_VERSION))
!         ereport(ERROR,
!                 (errmsg("multiple Python libraries are present in session"),
!                  errdetail("Only one Python major version can be used in one session.")));
!
!     /* The rest should only be done once per session */
!     if (inited)
!         return;

  #if PY_MAJOR_VERSION >= 3
      PyImport_AppendInittab("plpy", PyInit_plpy);
*************** _PG_init(void)
*** 120,126 ****
  }

  /*
!  * This should only be called once from _PG_init. Initialize the Python
   * interpreter and global data.
   */
  static void
--- 173,179 ----
  }

  /*
!  * This should be called only once, from PLy_initialize. Initialize the Python
   * interpreter and global data.
   */
  static void
*************** plpython_validator(PG_FUNCTION_ARGS)
*** 155,163 ****
          PG_RETURN_VOID();

      if (!check_function_bodies)
-     {
          PG_RETURN_VOID();
!     }

      /* Get the new function's pg_proc entry */
      tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcoid));
--- 208,217 ----
          PG_RETURN_VOID();

      if (!check_function_bodies)
          PG_RETURN_VOID();
!
!     /* Do this only after making sure we need to do something */
!     PLy_initialize();

      /* Get the new function's pg_proc entry */
      tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcoid));
*************** plpython_call_handler(PG_FUNCTION_ARGS)
*** 191,196 ****
--- 245,252 ----
      PLyExecutionContext *exec_ctx;
      ErrorContextCallback plerrcontext;

+     PLy_initialize();
+
      /* Note: SPI_finish() happens in plpy_exec.c, which is dubious design */
      if (SPI_connect() != SPI_OK_CONNECT)
          elog(ERROR, "SPI_connect failed");
*************** plpython_inline_handler(PG_FUNCTION_ARGS
*** 266,271 ****
--- 322,329 ----
      PLyExecutionContext *exec_ctx;
      ErrorContextCallback plerrcontext;

+     PLy_initialize();
+
      /* Note: SPI_finish() happens in plpy_exec.c, which is dubious design */
      if (SPI_connect() != SPI_OK_CONNECT)
          elog(ERROR, "SPI_connect failed");