Milter-greylist

On Thu, Sep 27, 2007 at 05:38:59AM +0200, manu@... wrote:
> Emmanuel Dreyfus <manu@...> wrote:
> 
> > A simple way of fixing the issue would just be to issue a TEMPFAIL in
> > this situation, which would reflect the real problem: your system cannot
> > function proprely because of resource exhaustion, and the sender should
> > retry later.
> 
> Wait, wait, wait... fopen/fdopen usage is quite limited:
> 1) dump.c, for duming the database
> 2) sync.c, for MX sync
> 3) milter-greylist.c, for the PID file
> 4) stat.c, for the custom stat output

you forget
 5) conf.c 
	which is used on startup *and* on every change on greylist.conf

> 
> #3 and #4 are not a problem: it happens on startup and you'll never use
> a file descriptor above 256 here
> 
> #2 can happen if the MX sync connection breaks during operation
> #1 is what Solaris users experience

The problem is not the frequency of passing the above code, but the 
fact, that in multithreading the descriptor numbers are counting quickly
for beyond 256 even if free descriptor below 256 exists (im fd limit is
above 256). There maybe only a small time slot at startup (as long the
milter engine is not running) where one can rely on file descriptor values
below 256 ...

> 
> The TEMPFAIL solution won't help here, but we can make sure we keep a

I agree.

> file descriptor < 256 open for dumps: after a dumps succeeds, we open
> /dev/null and use dup2 to associate it to the low file descriptor we
> just used. On the next dump, we open the new file, use dup2 and here we
> are. I quickly wrote some code for that idea just below (not tested, not
> built)
> 
> Opinions?

There are serveral problem in this particular proposal (I have already
running into while coping which this problem in the past):

1. dumpfd initialisation (using /dev/null) is to late - until the first
	call to dump the probability to get a descriptor below 256 my be
	fairly bad ...

2. If we assuming dumpfd containes a value of less than 256, duplication
	to the reserved desciptor (dup2) works fine the first time. But
	with fclose() this descriptor is closed also! It is *not*
	possible to keep the descriptor, because fdopen() and fclose()
	are not symmetric in this manner. There would be a function
	needed, say fdclose(), which gives the descriptor back (only
	detach the descriptor from the stream) but alas no such thing is
	existing :(

	So after one run of dump the low valued desriptor is lost and
	you cannot guarantee to reliably get a low descriptor again.
	Maybe you can loop a with short waits on trying to get a low
	valued descriptor - but this is not deterministic. When you
	catch one and if this is done in dump.c context maybe you miss
	the next dump intervall :(

	BTW this has been discussed in some ways in
	the thread around message
	http://tech.groups.yahoo.com/group/milter-greylist/message/3444

That are the reason why I put the file_ext.c module (as called the
Solaris workaround) into live (based on the suggestion in the above
mentioned thread), which seems to be complicated - but it has real
technical reasons why it has to be like it is (someone may proof or show
it to me or us to make it simpler). This module is already available -
not necessary to reinvent the wheel - and it replaces the
fopen/fdopen/fclose interface which let drop in drop in the code nearly
transparently:

 - replace all fopen/fdopen/fclose with Fopen/Fdopen/Fclose,
 - call a initialisation routine at startup, 
 - add one #include in a include-file which a read in by every module, 
 - set a define during compile, 
 - add the file_ext module for linking

and your done. ;) This way solves all file handle issues in whichever
module these are located - you will not have to insert workarounds again
and again depending on how much danger a file handle code is causing.
This is just a matter of maintainance ... (you have only to ensure to
use F* calls insteat of f* calls - should be easy)


Johann