From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: Michael Paquier <michael@paquier.xyz>,
        Justin Pryzby <pryzby@telsasoft.com>,
        PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: ENOSPC FailedAssertion("!(RefCountErrors == 0)"
In-reply-to: <20436.1531751966@sss.pgh.pa.us>
References: <20180627233939.GA10276@telsasoft.com>
 <20180628020432.GB11054@paquier.xyz> <7317.1531694923@sss.pgh.pa.us>
 <20180716131337.usestwjviwwocpyz@alap3.anarazel.de>
 <20436.1531751966@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Mon, 16 Jul 2018 10:39:26 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <4714.1531773064.0@sss.pgh.pa.us>
Date: Mon, 16 Jul 2018 16:36:51 -0400
Message-ID: <4993.1531773411@sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <4714.1531773064.1@sss.pgh.pa.us>

I wrote:
> So I said I didn't want to do extra work on this, but I am looking into
> fixing it by having these aux process types run a ResourceOwner that can
> be told to clean up any open buffer pins at exit.  We could be sure the
> coverage is complete by dint of removing the special-case code in
> resowner.c that allows buffer pins to be taken with no active resowner.
> Then CheckForBufferLeaks can be left as-is, ie something we do only
> in assert builds.

That turned out to be a larger can of worms than I'd anticipated, as it
soon emerged that we'd acquired a whole lot of cargo-cult programming
around ResourceOwners.  Having a ResourceOwner in itself does nothing
for you: there has to be code someplace that ensures we'll call
ResourceOwnerRelease at an appropriate time.  There was basically noplace
outside xact.c and portalmem.c that got this completely right.  bgwriter.c
and a couple of other places at least tried a little, by doing a
ResourceOwnerRelease in their sigsetjmp-catching stanzas, but that didn't
account for the process-exit code path.  Other places just created a
ResourceOwner and did *nothing* about cleaning it up.

I decided that the most expedient way of dealing with this was to create
a self-contained facility in resowner.c that would create a standalone
ResourceOwner and register a shmem-exit callback to clean it up.
That way it's easier (less code) to do it right than to do it wrong.
That led to the attached, which passes check-world as well as Michael's
full-disk test script.

I'm mostly pretty happy with this, but I think there are a couple of
loose ends in logicalfuncs.c and slotfuncs.c: those are creating
non-standalone ResourceOwners (children of whatever the active
ResourceOwner is) and doing nothing much to clean them up.  That seems
pretty bogus.  It's not a permanent resource leak, because sooner or
later the parent ResourceOwner will get cleaned up and that will
recurse to the child ... but then why bother with a child ResourceOwner
at all?  I added asserts to these calls to verify that there was a
parent resowner (if there isn't, the code is just broken), but I think
that we should either add more code to clean up the child resowner
promptly, or just not bother with a child at all.

Not very sure what to do with this.  I don't think we should try to
backpatch it, because it's essentially changing the API requirements
around buffer access in auxiliary processes --- but it might not be
too late to squeeze it into v11.  It is a bug fix, in that the current
code can leak buffer pins in some code paths and we won't notice in
non-assert builds; but we've not really seen any field reports of that
happening, so I'm not sure how important this is.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="use-resowner-in-all-aux-processes-1.patch";
	charset="us-ascii"
Content-ID: <4714.1531773064.2@sss.pgh.pa.us>
Content-Description: use-resowner-in-all-aux-processes-1.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/tr=
ansam/parallel.c
index 4e32cff..3a5cd0e 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -1221,8 +1221,7 @@ ParallelWorkerMain(Datum main_arg)
 	memcpy(&ParallelWorkerNumber, MyBgworkerEntry->bgw_extra, sizeof(int));
 =

 	/* Set up a memory context and resource owner. */
-	Assert(CurrentResourceOwner =3D=3D NULL);
-	CurrentResourceOwner =3D ResourceOwnerCreate(NULL, "parallel toplevel");
+	CreateAuxProcessResourceOwner();
 	CurrentMemoryContext =3D AllocSetContextCreate(TopMemoryContext,
 												 "Parallel worker",
 												 ALLOCSET_DEFAULT_SIZES);
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transa=
m/xlog.c
index 4049deb..5d01edd 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6356,6 +6356,15 @@ StartupXLOG(void)
 	struct stat st;
 =

 	/*
+	 * We should have an aux process resource owner to use, and we should no=
t
+	 * be in a transaction that's installed some other resowner.
+	 */
+	Assert(AuxProcessResourceOwner !=3D NULL);
+	Assert(CurrentResourceOwner =3D=3D NULL ||
+		   CurrentResourceOwner =3D=3D AuxProcessResourceOwner);
+	CurrentResourceOwner =3D AuxProcessResourceOwner;
+
+	/*
 	 * Verify XLOG status looks valid.
 	 */
 	if (ControlFile->state < DB_SHUTDOWNED ||
@@ -8467,6 +8476,15 @@ GetNextXidAndEpoch(TransactionId *xid, uint32 *epoc=
h)
 void
 ShutdownXLOG(int code, Datum arg)
 {
+	/*
+	 * We should have an aux process resource owner to use, and we should no=
t
+	 * be in a transaction that's installed some other resowner.
+	 */
+	Assert(AuxProcessResourceOwner !=3D NULL);
+	Assert(CurrentResourceOwner =3D=3D NULL ||
+		   CurrentResourceOwner =3D=3D AuxProcessResourceOwner);
+	CurrentResourceOwner =3D AuxProcessResourceOwner;
+
 	/* Don't be chatty in standalone mode */
 	ereport(IsPostmasterEnvironment ? LOG : NOTICE,
 			(errmsg("shutting down")));
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/boo=
tstrap.c
index 7e34bee..cdd71a9 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -403,6 +403,13 @@ AuxiliaryProcessMain(int argc, char *argv[])
 		/* finish setting up bufmgr.c */
 		InitBufferPoolBackend();
 =

+		/*
+		 * Auxiliary processes don't run transactions, but they may need a
+		 * resource owner anyway to manage buffer pins acquired outside
+		 * transactions (and, perhaps, other things in future).
+		 */
+		CreateAuxProcessResourceOwner();
+
 		/* Initialize backend status information */
 		pgstat_initialize();
 		pgstat_bestart();
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/=
autovacuum.c
index 78e4c85..1d9cfc6 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -522,13 +522,9 @@ AutoVacLauncherMain(int argc, char *argv[])
 		pgstat_report_wait_end();
 		AbortBufferIO();
 		UnlockBuffers();
-		if (CurrentResourceOwner)
-		{
-			ResourceOwnerRelease(CurrentResourceOwner,
-								 RESOURCE_RELEASE_BEFORE_LOCKS,
-								 false, true);
-			/* we needn't bother with the other ResourceOwnerRelease phases */
-		}
+		/* this is probably dead code, but let's be safe: */
+		if (AuxProcessResourceOwner)
+			ReleaseAuxProcessResources(false);
 		AtEOXact_Buffers(false);
 		AtEOXact_SMgr();
 		AtEOXact_Files(false);
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bg=
writer.c
index d5ce685..960d3de 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -142,12 +142,6 @@ BackgroundWriterMain(void)
 	sigdelset(&BlockSig, SIGQUIT);
 =

 	/*
-	 * Create a resource owner to keep track of our resources (currently onl=
y
-	 * buffer pins).
-	 */
-	CurrentResourceOwner =3D ResourceOwnerCreate(NULL, "Background Writer");
-
-	/*
 	 * We just started, assume there has been either a shutdown or
 	 * end-of-recovery snapshot.
 	 */
@@ -191,11 +185,7 @@ BackgroundWriterMain(void)
 		ConditionVariableCancelSleep();
 		AbortBufferIO();
 		UnlockBuffers();
-		/* buffer pins are released here: */
-		ResourceOwnerRelease(CurrentResourceOwner,
-							 RESOURCE_RELEASE_BEFORE_LOCKS,
-							 false, true);
-		/* we needn't bother with the other ResourceOwnerRelease phases */
+		ReleaseAuxProcessResources(false);
 		AtEOXact_Buffers(false);
 		AtEOXact_SMgr();
 		AtEOXact_Files(false);
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaste=
r/checkpointer.c
index 0950ada..de1b22d 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -232,12 +232,6 @@ CheckpointerMain(void)
 	last_checkpoint_time =3D last_xlog_switch_time =3D (pg_time_t) time(NULL=
);
 =

 	/*
-	 * Create a resource owner to keep track of our resources (currently onl=
y
-	 * buffer pins).
-	 */
-	CurrentResourceOwner =3D ResourceOwnerCreate(NULL, "Checkpointer");
-
-	/*
 	 * Create a memory context that we will do all our work in.  We do this =
so
 	 * that we can reset the context during error recovery and thereby avoid
 	 * possible memory leaks.  Formerly this code just ran in
@@ -275,11 +269,7 @@ CheckpointerMain(void)
 		pgstat_report_wait_end();
 		AbortBufferIO();
 		UnlockBuffers();
-		/* buffer pins are released here: */
-		ResourceOwnerRelease(CurrentResourceOwner,
-							 RESOURCE_RELEASE_BEFORE_LOCKS,
-							 false, true);
-		/* we needn't bother with the other ResourceOwnerRelease phases */
+		ReleaseAuxProcessResources(false);
 		AtEOXact_Buffers(false);
 		AtEOXact_SMgr();
 		AtEOXact_Files(false);
diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/w=
alwriter.c
index 688d5b5..eceed1b 100644
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -130,12 +130,6 @@ WalWriterMain(void)
 	sigdelset(&BlockSig, SIGQUIT);
 =

 	/*
-	 * Create a resource owner to keep track of our resources (not clear tha=
t
-	 * we need this, but may as well have one).
-	 */
-	CurrentResourceOwner =3D ResourceOwnerCreate(NULL, "Wal Writer");
-
-	/*
 	 * Create a memory context that we will do all our work in.  We do this =
so
 	 * that we can reset the context during error recovery and thereby avoid
 	 * possible memory leaks.  Formerly this code just ran in
@@ -172,11 +166,7 @@ WalWriterMain(void)
 		pgstat_report_wait_end();
 		AbortBufferIO();
 		UnlockBuffers();
-		/* buffer pins are released here: */
-		ResourceOwnerRelease(CurrentResourceOwner,
-							 RESOURCE_RELEASE_BEFORE_LOCKS,
-							 false, true);
-		/* we needn't bother with the other ResourceOwnerRelease phases */
+		ReleaseAuxProcessResources(false);
 		AtEOXact_Buffers(false);
 		AtEOXact_SMgr();
 		AtEOXact_Files(false);
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/=
replication/logical/logicalfuncs.c
index 54c25f1..6fac37b 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -279,6 +279,10 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fci=
nfo, bool confirm, bool bin
 		 */
 		startptr =3D MyReplicationSlot->data.restart_lsn;
 =

+		/*
+		 * Record our resource usage in a child of the current resowner.
+		 */
+		Assert(CurrentResourceOwner !=3D NULL);
 		CurrentResourceOwner =3D ResourceOwnerCreate(CurrentResourceOwner, "log=
ical decoding");
 =

 		/* invalidate non-timetravel entries */
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replic=
ation/logical/worker.c
index 0d2b795..62c7d66 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -1602,9 +1602,8 @@ ApplyWorkerMain(Datum main_arg)
 	/* Load the libpq-specific functions */
 	load_file("libpqwalreceiver", false);
 =

-	Assert(CurrentResourceOwner =3D=3D NULL);
-	CurrentResourceOwner =3D ResourceOwnerCreate(NULL,
-											   "logical replication apply");
+	/* Set up resource owner */
+	CreateAuxProcessResourceOwner();
 =

 	/* Run as replica session replication role. */
 	SetConfigOption("session_replication_role", "replica",
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication=
/slotfuncs.c
index 450f737..f2985ef 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -365,6 +365,10 @@ pg_logical_replication_slot_advance(XLogRecPtr moveto=
)
 									logical_read_local_xlog_page,
 									NULL, NULL, NULL);
 =

+		/*
+		 * Record our resource usage in a child of the current resowner.
+		 */
+		Assert(CurrentResourceOwner !=3D NULL);
 		CurrentResourceOwner =3D ResourceOwnerCreate(CurrentResourceOwner,
 												   "logical decoding");
 =

diff --git a/src/backend/replication/walreceiver.c b/src/backend/replicati=
on/walreceiver.c
index 987bb84..7c292d8 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -292,12 +292,6 @@ WalReceiverMain(void)
 	if (WalReceiverFunctions =3D=3D NULL)
 		elog(ERROR, "libpqwalreceiver didn't initialize correctly");
 =

-	/*
-	 * Create a resource owner to keep track of our resources (not clear tha=
t
-	 * we need this, but may as well have one).
-	 */
-	CurrentResourceOwner =3D ResourceOwnerCreate(NULL, "Wal Receiver");
-
 	/* Unblock signals (they were blocked when the postmaster forked us) */
 	PG_SETMASK(&UnBlockSig);
 =

diff --git a/src/backend/replication/walsender.c b/src/backend/replication=
/walsender.c
index 3a0106b..8e9e47b 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -265,7 +265,7 @@ InitWalSender(void)
 	InitWalSenderSlot();
 =

 	/* Set up resource owner */
-	CurrentResourceOwner =3D ResourceOwnerCreate(NULL, "walsender top-level =
resource owner");
+	CreateAuxProcessResourceOwner();
 =

 	/*
 	 * Let postmaster know that we're a WAL sender. Once we've declared us a=
s
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/po=
stinit.c
index 09e0df2..5ef6315 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -632,8 +632,19 @@ InitPostgres(const char *in_dbname, Oid dboid, const =
char *username,
 		 * We are either a bootstrap process or a standalone backend. Either
 		 * way, start up the XLOG machinery, and register to have it closed
 		 * down at exit.
+		 *
+		 * We don't yet have an aux-process resource owner, but StartupXLOG
+		 * and ShutdownXLOG will need one.  Hence, create said resource owner
+		 * (and register a callback to clean it up after ShutdownXLOG runs).
 		 */
+		CreateAuxProcessResourceOwner();
+
 		StartupXLOG();
+		/* Release (and warn about) any buffer pins leaked in StartupXLOG */
+		ReleaseAuxProcessResources(true);
+		/* Reset CurrentResourceOwner to nothing for the moment */
+		CurrentResourceOwner =3D NULL;
+
 		on_shmem_exit(ShutdownXLOG, 0);
 	}
 =

diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/res=
owner/resowner.c
index bce021e..c708ebd 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -22,6 +22,7 @@
 =

 #include "access/hash.h"
 #include "jit/jit.h"
+#include "storage/ipc.h"
 #include "storage/predicate.h"
 #include "storage/proc.h"
 #include "utils/memutils.h"
@@ -140,6 +141,7 @@ typedef struct ResourceOwnerData
 ResourceOwner CurrentResourceOwner =3D NULL;
 ResourceOwner CurTransactionResourceOwner =3D NULL;
 ResourceOwner TopTransactionResourceOwner =3D NULL;
+ResourceOwner AuxProcessResourceOwner =3D NULL;
 =

 /*
  * List of add-on callbacks for resource releasing
@@ -165,6 +167,7 @@ static void ResourceOwnerReleaseInternal(ResourceOwner=
 owner,
 							 ResourceReleasePhase phase,
 							 bool isCommit,
 							 bool isTopLevel);
+static void ReleaseAuxProcessResourcesCallback(int code, Datum arg);
 static void PrintRelCacheLeakWarning(Relation rel);
 static void PrintPlanCacheLeakWarning(CachedPlan *plan);
 static void PrintTupleDescLeakWarning(TupleDesc tupdesc);
@@ -823,6 +826,60 @@ UnregisterResourceReleaseCallback(ResourceReleaseCall=
back callback, void *arg)
 	}
 }
 =

+/*
+ * Establish an AuxProcessResourceOwner for the current process.
+ */
+void
+CreateAuxProcessResourceOwner(void)
+{
+	Assert(AuxProcessResourceOwner =3D=3D NULL);
+	Assert(CurrentResourceOwner =3D=3D NULL);
+	AuxProcessResourceOwner =3D ResourceOwnerCreate(NULL, "AuxiliaryProcess"=
);
+	CurrentResourceOwner =3D AuxProcessResourceOwner;
+
+	/*
+	 * Register a shmem-exit callback for cleanup of aux-process resource
+	 * owner.  (This needs to run after, e.g., ShutdownXLOG.)
+	 */
+	on_shmem_exit(ReleaseAuxProcessResourcesCallback, 0);
+
+}
+
+/*
+ * Convenience routine to release all resources tracked in
+ * AuxProcessResourceOwner (but that resowner is not destroyed here).
+ * Warn about leaked resources if isCommit is true.
+ */
+void
+ReleaseAuxProcessResources(bool isCommit)
+{
+	/*
+	 * At this writing, the only thing that could actually get released is
+	 * buffer pins; but we may as well do the full release protocol.
+	 */
+	ResourceOwnerRelease(AuxProcessResourceOwner,
+						 RESOURCE_RELEASE_BEFORE_LOCKS,
+						 isCommit, true);
+	ResourceOwnerRelease(AuxProcessResourceOwner,
+						 RESOURCE_RELEASE_LOCKS,
+						 isCommit, true);
+	ResourceOwnerRelease(AuxProcessResourceOwner,
+						 RESOURCE_RELEASE_AFTER_LOCKS,
+						 isCommit, true);
+}
+
+/*
+ * Shmem-exit callback for the same.
+ * Warn about leaked resources if process exit code is zero (ie normal).
+ */
+static void
+ReleaseAuxProcessResourcesCallback(int code, Datum arg)
+{
+	bool		isCommit =3D (code =3D=3D 0);
+
+	ReleaseAuxProcessResources(isCommit);
+}
+
 =

 /*
  * Make sure there is room for at least one more entry in a ResourceOwner=
's
@@ -830,15 +887,10 @@ UnregisterResourceReleaseCallback(ResourceReleaseCal=
lback callback, void *arg)
  *
  * This is separate from actually inserting an entry because if we run ou=
t
  * of memory, it's critical to do so *before* acquiring the resource.
- *
- * We allow the case owner =3D=3D NULL because the bufmgr is sometimes in=
voked
- * outside any transaction (for example, during WAL recovery).
  */
 void
 ResourceOwnerEnlargeBuffers(ResourceOwner owner)
 {
-	if (owner =3D=3D NULL)
-		return;
 	ResourceArrayEnlarge(&(owner->bufferarr));
 }
 =

@@ -846,29 +898,19 @@ ResourceOwnerEnlargeBuffers(ResourceOwner owner)
  * Remember that a buffer pin is owned by a ResourceOwner
  *
  * Caller must have previously done ResourceOwnerEnlargeBuffers()
- *
- * We allow the case owner =3D=3D NULL because the bufmgr is sometimes in=
voked
- * outside any transaction (for example, during WAL recovery).
  */
 void
 ResourceOwnerRememberBuffer(ResourceOwner owner, Buffer buffer)
 {
-	if (owner =3D=3D NULL)
-		return;
 	ResourceArrayAdd(&(owner->bufferarr), BufferGetDatum(buffer));
 }
 =

 /*
  * Forget that a buffer pin is owned by a ResourceOwner
- *
- * We allow the case owner =3D=3D NULL because the bufmgr is sometimes in=
voked
- * outside any transaction (for example, during WAL recovery).
  */
 void
 ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer)
 {
-	if (owner =3D=3D NULL)
-		return;
 	if (!ResourceArrayRemove(&(owner->bufferarr), BufferGetDatum(buffer)))
 		elog(ERROR, "buffer %d is not owned by resource owner %s",
 			 buffer, owner->name);
diff --git a/src/include/utils/resowner.h b/src/include/utils/resowner.h
index fe7f491..fa03942 100644
--- a/src/include/utils/resowner.h
+++ b/src/include/utils/resowner.h
@@ -33,6 +33,7 @@ typedef struct ResourceOwnerData *ResourceOwner;
 extern PGDLLIMPORT ResourceOwner CurrentResourceOwner;
 extern PGDLLIMPORT ResourceOwner CurTransactionResourceOwner;
 extern PGDLLIMPORT ResourceOwner TopTransactionResourceOwner;
+extern PGDLLIMPORT ResourceOwner AuxProcessResourceOwner;
 =

 /*
  * Resource releasing is done in three phases: pre-locks, locks, and
@@ -78,5 +79,7 @@ extern void RegisterResourceReleaseCallback(ResourceRele=
aseCallback callback,
 								void *arg);
 extern void UnregisterResourceReleaseCallback(ResourceReleaseCallback cal=
lback,
 								  void *arg);
+extern void CreateAuxProcessResourceOwner(void);
+extern void ReleaseAuxProcessResources(bool isCommit);
 =

 #endif							/* RESOWNER_H */
diff --git a/src/test/modules/test_shm_mq/worker.c b/src/test/modules/test=
_shm_mq/worker.c
index bcb992e..e9bc30d 100644
--- a/src/test/modules/test_shm_mq/worker.c
+++ b/src/test/modules/test_shm_mq/worker.c
@@ -75,7 +75,7 @@ test_shm_mq_main(Datum main_arg)
 	 * of contents so we can locate the various data structures we'll need t=
o
 	 * find within the segment.
 	 */
-	CurrentResourceOwner =3D ResourceOwnerCreate(NULL, "test_shm_mq worker")=
;
+	CreateAuxProcessResourceOwner();
 	seg =3D dsm_attach(DatumGetInt32(main_arg));
 	if (seg =3D=3D NULL)
 		ereport(ERROR,

------- =_aaaaaaaaaa0--