Mailing List Archive

dbmail-3.2.0 double lock?
Hi,

ci_read_cb() calls client->cb_error() under PLOCK(client->lock).

cb_error == client_error_cb, which also calls client->PLOCK(client->lock).

Can it hangs here in case of error?
_______________________________________________
Dbmail-dev mailing list
Dbmail-dev@dbmail.org
http://mailman.fastxs.nl/cgi-bin/mailman/listinfo/dbmail-dev
Re: dbmail-3.2.0 double lock? [ In reply to ]
At Tue, 02 Sep 2014 21:07:50 +0400,
Sergej Pupykin <ml@sergej.pp.ru> wrote:
> ci_read_cb() calls client->cb_error() under PLOCK(client->lock).
>
> cb_error == client_error_cb, which also calls client->PLOCK(client->lock).
>
> Can it hangs here in case of error?

I've made completely incorrect fix, which works and proofs that this
is the bug:

--- dbmail-3.2.0/src/clientbase.c 2014-08-23 17:01:38.000000000 +0400
+++ dbmail-3.2.0.my/src/clientbase.c 2014-09-02 21:22:01.075125889 +0400
@@ -413,11 +413,11 @@
break;

} else if (t == 0) {
- PLOCK(client->lock);
if (client->sock->ssl) {
if (client->cb_error(client->rx, t, (void *)client))
client->client_state |= CLIENT_ERR;
}
+ PLOCK(client->lock);
if (client->sock->ssl || client->rx) // EOF on stdin is not an error
client->client_state |= CLIENT_EOF;
PUNLOCK(client->lock);
_______________________________________________
Dbmail-dev mailing list
Dbmail-dev@dbmail.org
http://mailman.fastxs.nl/cgi-bin/mailman/listinfo/dbmail-dev
Re: dbmail-3.2.0 double lock? [ In reply to ]
You are right. The purpose of that lock is to protect race conditions on
client->client_state.

I'll push fix for this. Please let me know if you agree it fixes this.



On 03-09-14 11:15, Sergej Pupykin wrote:
>
> At Tue, 02 Sep 2014 21:07:50 +0400,
> Sergej Pupykin <ml@sergej.pp.ru> wrote:
>> ci_read_cb() calls client->cb_error() under PLOCK(client->lock).
>>
>> cb_error == client_error_cb, which also calls client->PLOCK(client->lock).
>>
>> Can it hangs here in case of error?
>
> I've made completely incorrect fix, which works and proofs that this
> is the bug:
>
> --- dbmail-3.2.0/src/clientbase.c 2014-08-23 17:01:38.000000000 +0400
> +++ dbmail-3.2.0.my/src/clientbase.c 2014-09-02 21:22:01.075125889 +0400
> @@ -413,11 +413,11 @@
> break;
>
> } else if (t == 0) {
> - PLOCK(client->lock);
> if (client->sock->ssl) {
> if (client->cb_error(client->rx, t, (void *)client))
> client->client_state |= CLIENT_ERR;
> }
> + PLOCK(client->lock);
> if (client->sock->ssl || client->rx) // EOF on stdin is not an error
> client->client_state |= CLIENT_EOF;
> PUNLOCK(client->lock);
> _______________________________________________
> Dbmail-dev mailing list
> Dbmail-dev@dbmail.org
> http://mailman.fastxs.nl/cgi-bin/mailman/listinfo/dbmail-dev
>

--
________________________________________________________________
Paul J Stevens pjstevns @ gmail, twitter, github, linkedin
www.nfg.nl/info@nfg.nl/+31.85.877.99.97
_______________________________________________
Dbmail-dev mailing list
Dbmail-dev@dbmail.org
http://mailman.fastxs.nl/cgi-bin/mailman/listinfo/dbmail-dev
Re: dbmail-3.2.0 double lock? [ In reply to ]
dbmail_3_2 branch looks working for me.

At Sun, 21 Sep 2014 17:01:10 +0200,
Paul J Stevens <paul@nfg.nl> wrote:
>
> You are right. The purpose of that lock is to protect race conditions on
> client->client_state.
>
> I'll push fix for this. Please let me know if you agree it fixes this.
>
>
>
> On 03-09-14 11:15, Sergej Pupykin wrote:
> >
> > At Tue, 02 Sep 2014 21:07:50 +0400,
> > Sergej Pupykin <ml@sergej.pp.ru> wrote:
> >> ci_read_cb() calls client->cb_error() under PLOCK(client->lock).
> >>
> >> cb_error == client_error_cb, which also calls client->PLOCK(client->lock).
> >>
> >> Can it hangs here in case of error?
> >
> > I've made completely incorrect fix, which works and proofs that this
> > is the bug:
> >
> > --- dbmail-3.2.0/src/clientbase.c 2014-08-23 17:01:38.000000000 +0400
> > +++ dbmail-3.2.0.my/src/clientbase.c 2014-09-02 21:22:01.075125889 +0400
> > @@ -413,11 +413,11 @@
> > break;
> >
> > } else if (t == 0) {
> > - PLOCK(client->lock);
> > if (client->sock->ssl) {
> > if (client->cb_error(client->rx, t, (void *)client))
> > client->client_state |= CLIENT_ERR;
> > }
> > + PLOCK(client->lock);
> > if (client->sock->ssl || client->rx) // EOF on stdin is not an error
> > client->client_state |= CLIENT_EOF;
> > PUNLOCK(client->lock);
> > _______________________________________________
> > Dbmail-dev mailing list
> > Dbmail-dev@dbmail.org
> > http://mailman.fastxs.nl/cgi-bin/mailman/listinfo/dbmail-dev
> >
>
> --
> ________________________________________________________________
> Paul J Stevens pjstevns @ gmail, twitter, github, linkedin
> www.nfg.nl/info@nfg.nl/+31.85.877.99.97
> _______________________________________________
> Dbmail-dev mailing list
> Dbmail-dev@dbmail.org
> http://mailman.fastxs.nl/cgi-bin/mailman/listinfo/dbmail-dev
_______________________________________________
Dbmail-dev mailing list
Dbmail-dev@dbmail.org
http://mailman.fastxs.nl/cgi-bin/mailman/listinfo/dbmail-dev