p_psTimeInit() allocates but doesn't free 274 memory blocks

[rhl@…]

Routines that call p_psTimeInit directly or indirectly allocate 274 persistent memory blocks,
that aren't labelled persistent.

Probably the simplest fix would be to define a function psTimeInit(void) that should be
called prior to science code; the memory system allows one to ask for leaks since a
given memBlockId. A better fix would be to declare all this memory persistent, but
this isn't currently possible (as it's down in psMetadata land). One approach would be
to allow a user to set the state of the memory code to persistent or transient; in which
case one could say;

const int is_persistent = p_psMemAllocatePersistent(true);
p_psTimeInit((p_psGetConfigFileName());
(void)p_psMemAllocatePersistent(is_persistent);

[rhl@…]

On 2006-06-07 RHL added

bool p_psMemAllocatePersistent(bool is_persistent);

to pslib. It should also be added to the SDRS.

I didn't add calls to this routine to p_psTimeInit(), although this should be done. Until
this _is_ done, users shouldn't be aware of this change (but now they can avoid the
leak in user space code)

[Paul Price]

Shouldn't there be a mutex lock around the operations in p_psMemAllocatePersistent?

[Paul Price]

Because we want to mark some complex structure as "persistent", and so that we
don't have to go through all its internals marking each pointer, we define a
global "persistent" flag which we turn on, then off?

I think this makes sense. I'm adding it to the SDRS.

[Paul Price]

Added to the SDRS:

Sometimes system code will need to allocate complex structures (such
as a \code{psMetadata}) that must all be marked as \code{persistent}.
To save this code the trouble of tracking down every allocated pointer
to mark each \code{persistent}, we define an additional private
function, \code{p_psMemAllocatePersistent}, that sets the use of
\code{persistent} within \code{psAlloc}. This allows us to call
\code{p_psMemAllocatePersistent(true)}, allocate all our persistent
memory, and then call \code{p_psMemAllocatePersistent(false)}. The
initial setting shall be for allocated memory not to be marked
\code{persistent}.
\begin{prototype}
bool p_psMemAllocatePersistent(bool is_persistent);
\end{prototype}

[Paul Price]

Added p_psMemAllocatePersistent() to p_psTimeInit(). Persistent is turned on at
the top, off before each "return false", and off at the end.

There are some calls to psError() that do not return from the function (!).
These I wrap in off/on calls, so that memory allocated by the error is not
marked persistent:

p_psMemAllocatePersistent(false);
psError(PS_ERR_BAD_PARAMETER_VALUE, true,

PS_ERRORTEXT_psTime_BAD_TABLE_COUNT, i+1, numTables);

p_psMemAllocatePersistent(true);

I think this is the best I can do without going through the code in detail.

Code compiles, but I havne't verified that the problem is fixed (would you mind,
since you are set up to do so?). Changes committed to CVS head.

[rhl@…]

There's no need to make the calls to psError non-persistent. The error stack is cleared (and freed)
whenever you call psError(..., true, ...) or psErrorClear().

Calls to psError with no error return are useless, I agree, but not a problem for the memory system.
(If you want a log message, call psLogMsg not psError).

[rhl@…]

Paul writes:

---

Shouldn't there be a mutex lock around the operations in p_psMemAllocatePersistent?

An interesting question. I don't think that the call is a risk to the integrity of the memory
system, but it does change the behaviour of all threads. I'm not sure how much this
worries me; it won't lead to leaks, just inconsistencies in whether leaks are detected. If
we want to do better, we'd need to make the persistent-by-default flag a per-thread variable.

[Paul Price]

Agreed. Can we close this bug?