Merge psFree with psMemDecrRefCounter functionality

[robert.desonia@…]

For reference-tracked memory management, psFree can be implicit upon a buffer
lossing all references to it (a la Java, etc.).

I suggest that psFree just call psMemDecrRefCounter and psMemDecrRefCounter to
be responsible for freeing buffers upon the last reference being removed.

[Paul Price]

More-detailed explaination of the relation between memory management and structures.

[Paul Price]

I'm not sure I understand this, but this was fleshed out a little more recently,
with the explicit specification of a ground rule:

The destructor for every structure should only call psFree if refCounter == 1;
otherwise, it decrements the reference counter and returns.

That is, it is the responsibility of the constructor/destructor to
increment/decrement the reference counter. This is necessary in order to allow
complicated structures, i.e. structures that include pointers to other structures.

I've attached a section from the revised SDRS that explains this in a bit more
detail.

[robert.desonia@…]

Yes, my suggestion is basically what seems to be in the revised/unreleased SDRS.

The old SDRS didn't explicitly spell out this, but this is how I implemented it

(except I put in a warning message given it was not clear that was perfectly
kosher).

Would to be OK to remove psMemDecrRefCounter altogether from the library given
the refined definition of psFree? It seems dangerous to dereference something
without checking if it should be freed before all references are lost.

[Paul Price]

Since one can imagine incrementing and decrementing reference counters outside
the operation of ps{Alloc,Free}, I don't believe psMemDecRefCounter should be
dropped.

[robert.desonia@…]

Just for clarification; you are stating that psMemDecrRefCounter shall not free
memory in which all references to it all removed?

[Paul Price]

psMemDecrRefCounter does not free memory; that is the job of psFree.
psMemDecrRefCounter decrements the reference counter associated with a block of
allocated memory. psFree frees the memory if and only if the reference counter
is unity; otherwise, it generates an error.

[robert.desonia@…]

I went back and read the new section more carefully, and I see now that you
pushed the responsibility to the higher-level functions to know how many
references there are and figure out when to call psMemDecrRefCounter vs. psFree.

I have concerns about thread safety in such a design as the action of checking
the reference count and performing the proper action can not be made atomic
easily. Pushing the rule of "last dereference frees the buffer" into
psFree/psMemDecrRefCounter makes this action atomic as this functions were made
thread-safe.

I guess I go back to my original recommendation that psFree should just
decrement reference until all references are accounted for, then actually free
the buffer.

[eugene]

sample code showing use of reference decrementing function, with error caught by psFree

[eugene]

example using psFree with merged DecrRef

[eugene]

I've been following this discussion, and I'll pipe in here now with my
thoughts. I'm not an expert on multithreaded coding, but if I
understand the process correctly, I think I can address that as well.

First, there are clear trade-offs between having a separate
functionality to decrement the reference counter independently of
psFree vs incorporating it into the functionality of psFree. If
psMemDecrRefCounter is part of psFree, the code is definitely shorter:
the free functions for rich structures just make the appropriate
psFree calls without worrying about the state of the incoming address
(if the address is NULL or (Nref == 0), psFree returns OK anyway; if
(Nref > 1), psFree decrements the counter; if (Nref == 1), psFree
frees the memory as well). On the other hand, if it is an error to
call psFree with memory for which Nref > 1, then the requirement that
all rich structures are freed using their specialized free functions
can be enforced (or at least violations can be caught). I've attached
two example codes which demonstrate the two concepts. I've also given
an example of the type of error which can be caught if psFree is not
allowed to free something with Nref > 1. The basic difference is
here (see the attached examples for the complete definitions).

/* the correct implementation */
main () {

Foo *foo = NULL;
Bar *bar = NULL;

foo = psFooAlloc (); Nref(foo) = 1
bar = psBarSet (foo); Nref(foo) = 2
psFooFree (foo); Nref(foo) = 1
psBarFree (bar); Nref(foo) = 0

}

/* bad code (memory allocated to foo leaks), detected by psFree (foo) */
main () {

Foo *foo = NULL;
Bar *bar = NULL;

foo = psFooAlloc (); Nref(foo) = 1
bar = psBarSet (foo); Nref(foo) = 2
psFree (foo); Nref(foo) = 1 returns an error
psFree (bar); Nref(foo) = 1 does not free foo

}

As for the issue of thread safety, I believe (but, please tell me if I
am wrong) that all memory Alloc / Free operations need to be performed
in only one thread at a time. You don't want two threads trying to
free the same block of data, nor do you want two threads trying to
decrement the same counter. I think this means that all functions
like psFooFree need to lock the mutex associated with the memory
blocks or memory counter before they operate on either the counter or
the memory. It also means that psFree and the psFooFree functions
need to use recursive locks so the thread can set the lock in the
psFooFree as well as in psFree called by psFooFree.

I guess the philosophical question is if the somewhat simpler coding
of the psFooFree functions and the thread locking is worth not having
the utility mechanism to catch some of the illegal psFree(foo); calls.
(Of course, the mechanism does not guarantee all psFree(foo) uses will
be caught; only those that actually cause a triggering memory leak).

[eugene]

I'm going to relent on this to move things along: let's merge the DecrRef into
the psFree function. This means we need to be on guard against psFree being
mis-used.

[Eric.VanAlst@…]

The resolution implies that psFree shall be used to deallocate all rich data
structures. If a data structure has several references, the reference counter
will be decremented. If the data structure has no references, the data
structure will be deallocated. There is a requirement in SDR-06 section 2.1.7
which states "psFree must generate an error if refCounter is not 1" which is in
conflict with the resolution.

Should the requirement be removed from the SDR?

[Paul Price]

Fixed in the SDRS:

"The memory
management routines respect the use of the \code{refCounter} field:
\code{psFree} will not free a block for which \code{refCounter} is not 1, but
shall decrement the \code{refCounter}, and \code{psAlloc} will initialize the
field to 1. However, these functions do not (and cannot practically) enforce
the use of the counters; this is a requirement of external APIs which intend to
use the feature."

[Paul Price]

Should be fixed in SDRS-07 and ADD-06 (7 September 2004).