Re: Reconnect a single connection used by multiple threads in embedded SQL in C application causes error. - Mailing list pgsql-bugs

On Fri, Mar 25, 2022 at 01:34:16PM +0000, egashira.yusuke@fujitsu.com wrote:
> > On Fri, Mar 04, 2022 at 11:49:39AM +0000, egashira.yusuke@fujitsu.com wrote:
> > > @@ -222,10 +222,15 @@ EXEC SQL CONNECT TO <replaceable>target</replaceable> <optional>AS <replaceable>
> > >    <para>
> > >     The <replaceable>connection-name</replaceable> is used to handle
> > >     multiple connections in one program.  It can be omitted if a
> > > -   program uses only one connection.  The most recently opened
> > > -   connection becomes the current connection, which is used by default
> > > -   when an SQL statement is to be executed (see later in this
> > > -   chapter).
> > > +   program uses only one connection.
> > > +  </para>
> > > +
> > > +  <para>
> > > +   The most recently opened connection becomes the current connection.
> > 
> > This is true.  Disconnecting also changes the current connection, to another
> > one of the open connections.  Please document that somewhere appropriate.
> 
> I have added that behavior to the "Closing a Connection" section.

I intended to ask you to document it such that the reader can predict which
connection becomes the current connection.  Just saying "another" doesn't give
the reader enough information to rely on any particular behavior.  Having said
that, I've changed my mind about documenting this.  The current behavior is
hard to use, particularly in a multithreaded program.  Also, we might want to
change it later.  Moreover, I think it's intuitive that use of current
connection is undefined if one closes current connection and does not later
initialize some connection.  Let's try to leave this disconnect matter
undocumented after all.

> --- a/doc/src/sgml/ecpg.sgml
> +++ b/doc/src/sgml/ecpg.sgml
> @@ -222,10 +222,18 @@ EXEC SQL CONNECT TO <replaceable>target</replaceable> <optional>AS <replaceable>
>    <para>
>     The <replaceable>connection-name</replaceable> is used to handle
>     multiple connections in one program.  It can be omitted if a
> -   program uses only one connection.  The most recently opened
> -   connection becomes the current connection, which is used by default
> -   when an SQL statement is to be executed (see later in this
> -   chapter).
> +   program uses only one connection.
> +  </para>
> +
> +  <para>
> +   The most recently opened connection becomes the current connection.
> +   If your application uses multiple threads, each thread has its own
> +   current connection, and it is the most recently opened connection
> +   in each thread.  Threads that have not yet opened the current
> +   connection treat the most recently opened connection in the
> +   application as the current connection.  The current connection is
> +   used by default when an SQL statement is to be executed (see later
> +   in this chapter).
>    </para>

Disconnects can make this false.

> @@ -435,10 +442,24 @@ EXEC SQL DISCONNECT <optional><replaceable>connection</replaceable></optional>;
>     closed.
>    </para>
>  
> +  <para>
> +   Disconnecting changes the current connection to another opened
> +   connection if the closed connection was the current connection.
> +  </para>
> +
>    <para>
>     It is good style that an application always explicitly disconnect
>     from every connection it opened.
>    </para>
> +
> +  <note>
> +    <para>
> +     Connections are shared among all threads.  Be careful not to
> +     disconnect a connection that is to be used in another thread.

In your original example, I suspect the program author would not think of the
closed connection as "to be used in" Thread#1.  I'm not sure what to write
here, but I feel this isn't it.  We could probably omit the two sentences
without replacing them.

> +     In particular, the behavior is undefined if a connection treated as
> +     the current connection in one thread is closed by another thread.

Contrary to this text, it's okay to close the global current connection, so
long as that connection is not the thread-specific current connection of any
thread other than the current thread.  (This is so awkward.)  It is safe even
though the connection in question would be the current connection of every
thread not having initiated a connection.  The "undefined if the current
thread is not the thread that opened" wording I suggested in my last message
was also flawed, because SET CONNECTION or a disconnect can cause the
thread-specific current connection to become a connection opened in a
different thread.

I'm surprised to find how hard it is to document this without introducing a
false statement.