You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "Asankha C. Perera" <as...@wso2.com> on 2008/08/29 14:44:55 UTC

Connection closes and SharedInputBuffer shutdown

Hi Oleg

I am looking at the SharedInputBuffer, for which Jason has proposed some 
fixes (https://issues.apache.org/jira/browse/HTTPCORE-172), with which I 
agree.

Sometime back my original fix for it, was to not shutdown the buffer, 
while it had data which was not yet read as:

public void shutdown() {
        if (this.shutdown) {
            return;
        }
        if (!hasData() && this.endOfStream) {
            this.shutdown = true;
            synchronized (this.mutex) {
                this.mutex.notifyAll();
            }
        }
    }

However, I think Jasons version of the fix as described in the above 
JIRA is better. Do you have any comments on these approaches?

However, even after the above version of the fix, Eric reported that he 
saw the same resultant error usually caused by this bug in his 
production environment.

I was looking at the following code:

public int consumeContent(final ContentDecoder decoder) throws IOException {
        if (this.shutdown) {
            return -1;
        }

and wondering why you have the above if condition.. is it even remotely 
possible for a consumeContent() call (triggered by an inputReady() to 
come after a closed() call? If so, we may need another fix here..

thanks
asankha

-- 
Asankha C. Perera

WSO2 - http://wso2.org
http://esbmagic.blogspot.com


Re: Connection closes and SharedInputBuffer shutdown

Posted by Oleg Kalnichevski <ol...@apache.org>.
On Fri, 2008-08-29 at 18:14 +0530, Asankha C. Perera wrote:
> Hi Oleg
> 
> I am looking at the SharedInputBuffer, for which Jason has proposed some 
> fixes (https://issues.apache.org/jira/browse/HTTPCORE-172), with which I 
> agree.
> 
> Sometime back my original fix for it, was to not shutdown the buffer, 
> while it had data which was not yet read as:
> 
> public void shutdown() {
>         if (this.shutdown) {
>             return;
>         }
>         if (!hasData() && this.endOfStream) {
>             this.shutdown = true;
>             synchronized (this.mutex) {
>                 this.mutex.notifyAll();
>             }
>         }
>     }
> 
> However, I think Jasons version of the fix as described in the above 
> JIRA is better. Do you have any comments on these approaches?
> 

Hi Asankha,

Yes, I posted my comments to the issue report. Generally, I am fine with
the proposed changes as long as synchronization of the threading unsafe
#hasData() method is taken care of. However, I have an alternative
solution to propose which preserves the existing semantics of the
#shutdown() method and solves the problem by adding new #close() method.
Please take a look. Ideally I would like to be able to differentiate
between an abnormal shutdown and an orderly closure cases.  


> However, even after the above version of the fix, Eric reported that he 
> saw the same resultant error usually caused by this bug in his 
> production environment.
> 
> I was looking at the following code:
> 
> public int consumeContent(final ContentDecoder decoder) throws IOException {
>         if (this.shutdown) {
>             return -1;
>         }
> 
> and wondering why you have the above if condition.. is it even remotely 
> possible for a consumeContent() call (triggered by an inputReady() to 
> come after a closed() call? If so, we may need another fix here..
> 

The initial idea was that buffers get shut down only in case of a
non-recoverable problem such as I/O failure, so their internal state
were no longer consistent / valid. If we change the semantics of the
#shutdown() method we may also need to change the above behavior.  

Cheers

Oleg


> thanks
> asankha
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@synapse.apache.org
For additional commands, e-mail: dev-help@synapse.apache.org


Re: Connection closes and SharedInputBuffer shutdown

Posted by Oleg Kalnichevski <ol...@apache.org>.
On Fri, 2008-08-29 at 18:14 +0530, Asankha C. Perera wrote:
> Hi Oleg
> 
> I am looking at the SharedInputBuffer, for which Jason has proposed some 
> fixes (https://issues.apache.org/jira/browse/HTTPCORE-172), with which I 
> agree.
> 
> Sometime back my original fix for it, was to not shutdown the buffer, 
> while it had data which was not yet read as:
> 
> public void shutdown() {
>         if (this.shutdown) {
>             return;
>         }
>         if (!hasData() && this.endOfStream) {
>             this.shutdown = true;
>             synchronized (this.mutex) {
>                 this.mutex.notifyAll();
>             }
>         }
>     }
> 
> However, I think Jasons version of the fix as described in the above 
> JIRA is better. Do you have any comments on these approaches?
> 

Hi Asankha,

Yes, I posted my comments to the issue report. Generally, I am fine with
the proposed changes as long as synchronization of the threading unsafe
#hasData() method is taken care of. However, I have an alternative
solution to propose which preserves the existing semantics of the
#shutdown() method and solves the problem by adding new #close() method.
Please take a look. Ideally I would like to be able to differentiate
between an abnormal shutdown and an orderly closure cases.  


> However, even after the above version of the fix, Eric reported that he 
> saw the same resultant error usually caused by this bug in his 
> production environment.
> 
> I was looking at the following code:
> 
> public int consumeContent(final ContentDecoder decoder) throws IOException {
>         if (this.shutdown) {
>             return -1;
>         }
> 
> and wondering why you have the above if condition.. is it even remotely 
> possible for a consumeContent() call (triggered by an inputReady() to 
> come after a closed() call? If so, we may need another fix here..
> 

The initial idea was that buffers get shut down only in case of a
non-recoverable problem such as I/O failure, so their internal state
were no longer consistent / valid. If we change the semantics of the
#shutdown() method we may also need to change the above behavior.  

Cheers

Oleg


> thanks
> asankha
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org