Milter-greylist

Hello,

I would like to provide a solution for a long standing problem.
On many platforms the final_dump() is not executed after
the milter-greylist has received a TERM signal.
With this patch the problem goes away,
at least on the Linux platform where I tested it.
On the Tru64 UNIX platform it was working before
and continues to work.

Bests,
Attila

At Sat, 7 Oct 2006 22:34:19 +0200,
<attila.bruncsak@...> wrote:
>
> I would like to provide a solution for a long standing problem.
> On many platforms the final_dump() is not executed after
> the milter-greylist has received a TERM signal.
> With this patch the problem goes away,
> at least on the Linux platform where I tested it.
> On the Tru64 UNIX platform it was working before
> and continues to work.
> 
Your patch is itself correct. Unfortunatelly, there are race
conditions in the libmilter causing abort() or SEGV. We cannot ensure
that the final_dump() always executed before the libmilter's bug is
also fixed. I hope sendmail 8.13.9 will include the following patch.
http://www.j10n.org/files/libmilter-8.13.8-signal.patch

AIDA Shinra <shinra@...> wrote:

> > I would like to provide a solution for a long standing problem.
> > On many platforms the final_dump() is not executed after
> > the milter-greylist has received a TERM signal.
> > With this patch the problem goes away,
> > at least on the Linux platform where I tested it.
> > On the Tru64 UNIX platform it was working before
> > and continues to work.
> > 
> Your patch is itself correct. Unfortunatelly, there are race
> conditions in the libmilter causing abort() or SEGV. We cannot ensure
> that the final_dump() always executed before the libmilter's bug is
> also fixed. I hope sendmail 8.13.9 will include the following patch.
> http://www.j10n.org/files/libmilter-8.13.8-signal.patch

So do you advise to let that patch apart until libmilter is fixed, or is
it better than nothing?

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...

At Sun, 8 Oct 2006 15:25:06 +0200,
manu@... wrote:
> 
> AIDA Shinra <shinra@...> wrote:
> 
> > > I would like to provide a solution for a long standing problem.
> > > On many platforms the final_dump() is not executed after
> > > the milter-greylist has received a TERM signal.
> > > With this patch the problem goes away,
> > > at least on the Linux platform where I tested it.
> > > On the Tru64 UNIX platform it was working before
> > > and continues to work.
> > > 
> > Your patch is itself correct. Unfortunatelly, there are race
> > conditions in the libmilter causing abort() or SEGV. We cannot ensure
> > that the final_dump() always executed before the libmilter's bug is
> > also fixed. I hope sendmail 8.13.9 will include the following patch.
> > http://www.j10n.org/files/libmilter-8.13.8-signal.patch
> 
> So do you advise to let that patch apart until libmilter is fixed, or is
> it better than nothing?

Better than nothing. No reason to postpone the patch.

<attila.bruncsak@...> wrote:

> I would like to provide a solution for a long standing problem.
> On many platforms the final_dump() is not executed after
> the milter-greylist has received a TERM signal.
> With this patch the problem goes away,
> at least on the Linux platform where I tested it.
> On the Tru64 UNIX platform it was working before
> and continues to work.

There is a problem with that patch: it violates the libmilter API. This says
that signals are to be handled by libmilter itself:
http://www.sendmail.org/doc/sendmail-current/libmilter/docs/overview.html

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...

At Sun, 8 Oct 2006 16:55:08 +0200,
manu@... wrote:
> 
> <attila.bruncsak@...> wrote:
> 
> > I would like to provide a solution for a long standing problem.
> > On many platforms the final_dump() is not executed after
> > the milter-greylist has received a TERM signal.
> > With this patch the problem goes away,
> > at least on the Linux platform where I tested it.
> > On the Tru64 UNIX platform it was working before
> > and continues to work.
> 
> There is a problem with that patch: it violates the libmilter API. This says
> that signals are to be handled by libmilter itself:
> http://www.sendmail.org/doc/sendmail-current/libmilter/docs/overview.html
> 
Anyway, libmilter blocks the signals in the smfi_main() thread. The
mask is inherited by all descendant threads. Libmilter does not catch
the signals but picks them by sigwait().

> > There is a problem with that patch: it violates the libmilter API. This says
> > that signals are to be handled by libmilter itself:
> > http://www.sendmail.org/doc/sendmail-current/libmilter/docs/overview.html
> > 

OK, attached an other patch, replacing the previous one.
This has absolutely no any reference to signal handling stuff.
BTW, by my opinion it is time now to update both the libmilter
code and documentation.

> Anyway, libmilter blocks the signals in the smfi_main() thread. The
> mask is inherited by all descendant threads. Libmilter does not catch
> the signals but picks them by sigwait().
>

At Mon, 9 Oct 2006 15:47:06 +0200,
<attila.bruncsak@...> wrote:
> 
> [1  <text/plain; ISO-8859-1 (7bit)>]
> > > There is a problem with that patch: it violates the libmilter API. This says
> > > that signals are to be handled by libmilter itself:
> > > http://www.sendmail.org/doc/sendmail-current/libmilter/docs/overview.html
> > > 
> 
> OK, attached an other patch, replacing the previous one.
> This has absolutely no any reference to signal handling stuff.
> BTW, by my opinion it is time now to update both the libmilter
> code and documentation.
> 
> > Anyway, libmilter blocks the signals in the smfi_main() thread. The
> > mask is inherited by all descendant threads. Libmilter does not catch
> > the signals but picks them by sigwait().
> > 

The new patch breaks our sync mechanism. The sync listener thread does
not start before a client connects to the server.

In contrast, I can't find any reason why the previous patch "break"s
the API. To be paranoid, we just need to unblock signals once exactly
before smfi_main().

> The new patch breaks our sync mechanism. The sync listener thread does
> not start before a client connects to the server.
> 
Already first connection from the client at the connect phase
(first call back form mlfi_main) will start the sync listener.
All the other connections will be blocked on the new mutex
which got introduced. There is no any difference with the situation
before.

On the other hand it was neither guaranteed before the patch
that the sync listener did any kind of useful thing before client
connections came in, since it is not a high priority
real-time thread.

Bests,
Attila

<attila.bruncsak@...> wrote:

> Already first connection from the client at the connect phase
> (first call back form mlfi_main) will start the sync listener.
> All the other connections will be blocked on the new mutex
> which got introduced. There is no any difference with the situation
> before.
> 
> On the other hand it was neither guaranteed before the patch
> that the sync listener did any kind of useful thing before client
> connections came in, since it is not a high priority
> real-time thread.

Perhaps we could introduce that after the 3.0 release, as we are not
sure the cure won't introduce unexpected new problems...

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...

At Tue, 10 Oct 2006 11:21:02 +0200,
<attila.bruncsak@...> wrote:
> 
> > The new patch breaks our sync mechanism. The sync listener thread does
> > not start before a client connects to the server.
> > 
> Already first connection from the client at the connect phase
> (first call back form mlfi_main) will start the sync listener.
> All the other connections will be blocked on the new mutex
> which got introduced. There is no any difference with the situation
> before.

Really? I think the sync_master_restart() call at main() launches
the master threads and they begin waiting connections from peers.

> On the other hand it was neither guaranteed before the patch
> that the sync listener did any kind of useful thing before client
> connections came in, since it is not a high priority
> real-time thread.

I meant we cannot receive sync messages until we receive a mail from
someone if we apply your second patch.

At Tue, 10 Oct 2006 13:52:40 +0200,
manu@... wrote:
> 
> <attila.bruncsak@...> wrote:
> 
> > Already first connection from the client at the connect phase
> > (first call back form mlfi_main) will start the sync listener.
> > All the other connections will be blocked on the new mutex
> > which got introduced. There is no any difference with the situation
> > before.
> > 
> > On the other hand it was neither guaranteed before the patch
> > that the sync listener did any kind of useful thing before client
> > connections came in, since it is not a high priority
> > real-time thread.
> 
> Perhaps we could introduce that after the 3.0 release, as we are not
> sure the cure won't introduce unexpected new problems...

I believe the first patch introduces no new problem. To be really
paranoid, we can unblock signals exactly before smfi_main() to allow
libmilter *catch* signals rather than probe by sigwait().

> > 
> > > The new patch breaks our sync mechanism. The sync 
> listener thread does
> > > not start before a client connects to the server.
> > > 
> > Already first connection from the client at the connect phase
> > (first call back form mlfi_main) will start the sync listener.
> > All the other connections will be blocked on the new mutex
> > which got introduced. There is no any difference with the situation
> > before.
> 
> Really? I think the sync_master_restart() call at main() launches
> the master threads and they begin waiting connections from peers.
> 

The sync_master_restart() will be called at the first client connection,
It will do the same stuff as it would be called fron main().
I do not see problem here.

> > On the other hand it was neither guaranteed before the patch
> > that the sync listener did any kind of useful thing before client
> > connections came in, since it is not a high priority
> > real-time thread.
> 
> I meant we cannot receive sync messages until we receive a mail from
> someone if we apply your second patch.
> 
Correct, this is the only difference. We cannot receive sync messages
until someone connects via the sendmail.

At Tue, 10 Oct 2006 15:14:21 +0200,
<attila.bruncsak@...> wrote:
> 
> > > 
> > > > The new patch breaks our sync mechanism. The sync 
> > listener thread does
> > > > not start before a client connects to the server.
> > > > 
> > > Already first connection from the client at the connect phase
> > > (first call back form mlfi_main) will start the sync listener.
> > > All the other connections will be blocked on the new mutex
> > > which got introduced. There is no any difference with the situation
> > > before.
> > 
> > Really? I think the sync_master_restart() call at main() launches
> > the master threads and they begin waiting connections from peers.
> > 
> 
> The sync_master_restart() will be called at the first client connection,
> It will do the same stuff as it would be called fron main().
> I do not see problem here.
> 
> > > On the other hand it was neither guaranteed before the patch
> > > that the sync listener did any kind of useful thing before client
> > > connections came in, since it is not a high priority
> > > real-time thread.
> > 
> > I meant we cannot receive sync messages until we receive a mail from
> > someone if we apply your second patch.
> > 
> Correct, this is the only difference. We cannot receive sync messages
> until someone connects via the sendmail.

We cannot receive sync messages until someone connects via the
sendmail of ONE OF PEERS. It may happens before your patched server
receive a mail.