Philip Martin wrote:
>
> I haven't looked at the lifetime of txn->pool, how long does it last?
>
IIRC, that's the same pool from the editor baton, so it lasts quite a
while, relatively speaking.
>
> Some of the existing error handling isn't correct. The run_hook_cmd
> function being an obvious example, in some places it explicitly cleans
> up but in other places it bypass the cleanup, it's inconsistent.
>
> In general SVN_ERR is OK, at least as far as memory goes. At some
> stage the error will be handled and at it seems reasonable to require
> that the code that handles the error should clear any related pools.
> Non-memory resources require more thought.
Thanks for the clarifications -- that does help and it would be a very
useful addition to the HACKING document, IMO...
>
>
>>Is the more explicit cleanup desired in this area because it's dealing
>>with pipes and files, or was it just the way it was done at the time
>>or something? Is it ok to create a subpool and use SVN_ERR to bail on
>>error conditions, or should I explicitly clear it before exiting if
>>any errors are encountered so that the files/pipes are guaranteed
>>closed even in the face of errors?
>
>
> Yes, it's the open files that are a particular worry. This function
> gets called by mod_dav_svn so it could be part of a multi-threaded
> server process. When that happens there may be multiple commits
> happening simultaneously in the same process, how many open files are
> going to exist simultaneously? What sort of limits exist?
>
> I suspect that the OS limit will usually be sufficiently high that
> this is not a problem in practice. It's possible I'm worrying about a
> problem that doesn't exist.
>
>
The pre-commit hook stuff (I think -- I'm not totally confident of my
understanding of the code involved), would last through network round
trips and perhaps others, too, if left to pool cleanup. So, I think
I'll revise it and ask for another round of feedback, if that's ok.
>>I'm not trying to be a bother, just clarify my understanding of how
>>thing should be done.
>
>
> Given the confused state of the current run_hook_cmd code, it
> may well be that your current patch is enough of an improvement to
> warrant committing it. I'm not vetoing it, I don't know for certain
> there is a problem, I'm just asking questions.
>
Sure, I understand and that's one of the things I like about Subversion
-- the quality that's expected, even if it can be a bit daunting at
times. :)
Thanks!
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Sep 6 02:05:36 2003