Hi Bongjae,
the fix looks good! Please go ahead and commit it (pls. don't forget
about the copyright years ;))
Thanks a lot!
WBR,
Alexey.
On 18.02.14 00:21, Bongjae Chang wrote:
> 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
>>>>>>>