Re: Strange Windows problem, lock_timeout test request

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> * Boszormenyi Zoltan (zb(at)cybertec(dot)at) wrote:
>> Stephen Frost was against the array pointer/count variant,
>> it was done that way earlier. Let me redo it again. :-)

> I still don't particularly like the array approach, and see the
> array+count approach as worse (seems like a higher chance that the count
> will end up being wrong at some point than having an array termination
> identifier). I still like the List approach, as that builds on a
> structure we've already got and can take advantage of the existing
> infrastructure. but Tom's got a good point regarding the potential for
> memory leaks with that solution.

> I havn't had a chance to look, but I would have expected the Lists for
> these to be allocated in a per-statement context, which would address
> the memory leak issue. Perhaps that isn't possible though. I agree
> that the List construct doesn't particularly help the callers, though I
> do think it makes the enable_timeouts() function cleaner.

I'm not sure about that. I think locks will typically get taken in
query-lifespan or transaction-lifespan contexts, so that repeated leaks
would be significant. Possibly my fear of leaks during error exits is
unfounded, but I think forgetting to pfree the list cells in normal
execution would be a problem.

Anyway, the bottom line for me was that the List way didn't seem to be
either convenient for the callers or especially efficient, because of
the need to palloc list cells and then remember to pfree them again.

Another way that we perhaps should consider is to follow the example of
XLogInsert and use internally-threaded lists that are typically stored
in local arrays in the callers. I've never thought that way was
especially beautiful, but it does have the advantage of being an idiom
that's already in use in other low-level code.

On the whole though, I don't see anything wrong with pointer-and-count.
I don't really believe that there's ever going to be a need to enable
more than a couple of timeouts simultaneously, so I don't want an overly
complicated data structure for it.

regards, tom lane