[Bps-public-commit] rt-extension-rest2 branch, no-shared-connections, created. c3610c1a6a2460be4fd0c7044c40137e588fec7a

[Alex Vandiver]

The branch, no-shared-connections has been created
        at  c3610c1a6a2460be4fd0c7044c40137e588fec7a (commit)

- Log -----------------------------------------------------------------
commit c3610c1a6a2460be4fd0c7044c40137e588fec7a
Author: Alex Vandiver <alex at chmrr.net>
Date:   Tue Nov 7 22:13:23 2017 -0500

    Connect to the database at request start, not app init
    
    rt-server takes pains to close down the database connection before
    handing off control to the PSGI server, such that the database
    connection is not preserved across the `fork`.  DB connections which
    are so shared result in undefined behavior -- postgres, for instance,
    will reuse the same statement ids on the handle in different
    processes, resulting in errors of the form:
    
        DBD::Pg::st execute failed: ERROR:  prepared statement "dbdpg_p4068_1" already exists
    
    Unfortunately, `to_app` is called _after_ that preventative
    disconnection has happened, so opening the database connection there
    _does_ cause it to be shared -- with the aforementioned disasterous
    results.
    
    _Some_ call to `ConnectToDatabase` is required, because (at the time
    of this commit, in <= 4.4) the only other explicit call to it is done
    inside the Mason handler.  Thus, without an explicit connect in this
    extension, REST requests which happen to have not served a Mason page
    will fail, due to lack of a database connection.
    
    Move database connections to inside the PSGI handlers themselves, such
    that the connection is opened inside the forked child, not the parent.
    Just the addition in Auth is technically sufficient, since it
    currently always runs before the main dispatcher -- however, we
    include the connection there as a preventative measure.  This is safe
    because DBIx::SearchBuilder::Handle's Connect (and thus
    RT::ConnectToDatabase) is a no-op if the connection is already live.

diff --git a/lib/RT/Extension/REST2.pm b/lib/RT/Extension/REST2.pm
index fdecd03..ad88726 100644
--- a/lib/RT/Extension/REST2.pm
+++ b/lib/RT/Extension/REST2.pm
@@ -566,8 +566,6 @@ sub to_psgi_app { shift->to_app(@_) }
 sub to_app {
     my $class = shift;
 
-    RT::ConnectToDatabase();
-
     return builder {
         enable '+RT::Extension::REST2::Middleware::ErrorAsJSON';
         enable '+RT::Extension::REST2::Middleware::Log';
diff --git a/lib/RT/Extension/REST2/Dispatcher.pm b/lib/RT/Extension/REST2/Dispatcher.pm
index 1c7f780..166492e 100644
--- a/lib/RT/Extension/REST2/Dispatcher.pm
+++ b/lib/RT/Extension/REST2/Dispatcher.pm
@@ -43,6 +43,8 @@ sub to_psgi_app {
 
     return sub {
         my $env = shift;
+
+        RT::ConnectToDatabase();
         my $dispatch = $self->_dispatcher->dispatch($env->{PATH_INFO});
 
         return [404, ['Content-Type' => 'text/plain'], 'Not Found']
diff --git a/lib/RT/Extension/REST2/Middleware/Auth.pm b/lib/RT/Extension/REST2/Middleware/Auth.pm
index 875ebcf..7e33738 100644
--- a/lib/RT/Extension/REST2/Middleware/Auth.pm
+++ b/lib/RT/Extension/REST2/Middleware/Auth.pm
@@ -14,6 +14,7 @@ our @auth_priority = qw(
 sub call {
     my ($self, $env) = @_;
 
+    RT::ConnectToDatabase();
     for my $method (@auth_priority) {
         last if $env->{'rt.current_user'} = $self->$method($env);
     }

-----------------------------------------------------------------------