Hi Alexey,
I tried to fix both 1) and 2) with another way simply.
When I investigated this more, I found the multipart processing should be
stopped if any errors were occurred.
Then, CompletionHandler was not needed anymore because
MultipartReadHandler#onError would call the CompletionHandler#failed
instead.
The NPE about 1) was similar to this.
I attached a proposed patch for this issue again.
Please review it again.
Thanks!
Regards,
Bongjae Chang
On 2/18/14, 2:04 PM, "Bongjae Chang" <bongjae.chang_at_gmail.com> wrote:
>Hi Alexey,
>1) I remembered I met NPE in NIOInputStreamImpl when I tested throwing
>Exception in ReadHandler. When I debugged it,
>NIOInputStreamImpl#recycle()
>was called during client packets were being received. If inputBuffer
>should never be null, maybe I would investigate it more.
>2) I agreed. I will rework about it again.
>
>Thanks!
>
>Regards,
>Bongjae Chang
>
>
>
>
>
>On 2/18/14, 12:31 PM, "Oleksiy Stashok" <oleksiy.stashok_at_oracle.com>
>wrote:
>
>>Hi Bongjae,
>>
>>thank you for the patch!
>>It looks good, just something wanted to check with you:
>>
>>1) do we really have to update existing NIOInputStreamImpl? IMO
>>inputBuffer should never be null
>>2) we can make CompletionHandler to be a part of MultipartContext, so it
>>will be accessible everywhere. We can make CompletionHandler attribute
>>protected or package private and don't expose it outside.
>>
>>What do you think?
>>
>>Thanks.
>>
>>WBR,
>>Alexey.
>>
>>On 13.02.14 01:14, Bongjae Chang wrote:
>>> Hi Alexey,
>>>
>>> I attached a proposed patch based on master branch about this issue.
>>>
>>> When I reviewed sources, MultipartEntryNIOInputStream should hold
>>>user¹s
>>> completion handler in order to call CompletionHandler#failed because we
>>> can't control the user¹s ReadHandler without wrapping it.
>>>
>>> Could you review it?
>>>
>>> Thanks!
>>>
>>> Regards,
>>> Bongjae Chang
>>>
>>>
>>>
>>>
>>>
>>> On 2/13/14, 12:04 PM, "Bongjae Chang" <bongjae.chang_at_gmail.com> wrote:
>>>
>>>> Hi Alexey,
>>>>
>>>> Ok. I will investigate it.
>>>>
>>>> Thanks.
>>>>
>>>> Regards,
>>>> Bongjae Chang
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On 2/13/14, 10:52 AM, "Oleksiy Stashok" <oleksiy.stashok_at_oracle.com>
>>>> wrote:
>>>>
>>>>> Hi Bongjae,
>>>>>
>>>>> I think it's a bug, CompletionHandler#failed has to be called.
>>>>> Do you have time to submit a fix?
>>>>>
>>>>> Thanks.
>>>>>
>>>>> WBR,
>>>>> Alexey.
>>>>>
>>>>>
>>>>> On 11.02.14 18:44, Bongjae Chang wrote:
>>>>>> Hi,
>>>>>>
>>>>>> When I tested multipart with http-multipart-samples sources, I met
>>>>>> some strange behavior.
>>>>>> I expected a failed notification of CompletionHandler if an error
>>>>>>was
>>>>>> occurred in processing multipart, but there was no signal in some
>>>>>>case.
>>>>>>
>>>>>> As you know, a user should implement two interfaces in order to call
>>>>>> MultipartScanner.scan() successfully.
>>>>>> 1. MultipartEntryHandler
>>>>>> 2. ReadHandler in MultipartEntryHandler
>>>>>>
>>>>>> If ReadHandler throws an exception in processing some works,
>>>>>> ReadHandler#onError() will be called. But user¹s
>>>>>> CompletionHandler#failed() was not called.
>>>>>>
>>>>>> You can reproduce this with samples easily like this.
>>>>>>
>>>>>> In UploaderHttpHandler.java
>>>>>> ‹
>>>>>> private void readAndSaveAvail() throws IOException {
>>>>>> while(inputStream.isReady()) {
>>>>>> final int readBytes = inputStream.read(buf);
>>>>>>
>>>>>> // limit max size for uploading a file.
>>>>>> final long currentBytes = uploadedBytesCounter.addAndGet(readBytes);
>>>>>> if (currentBytes >= 1024 * 1000) {
>>>>>> // throw exception for stopping upload
>>>>>> throw new IOException(³maximum size exceeded²);
>>>>>> }
>>>>>> ...
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> I would like to know this behavior is intended.
>>>>>> (In other words, I would like to know if user should call
>>>>>> CompletionHandler#failed directly in ReadHandler#onError.)
>>>>>>
>>>>>> Thanks!
>>>>>>
>>>>>> Regards,
>>>>>> Bongjae Chang
>>>>>>