RFR 8185719 [testlibrary] : RMI TestSocketFactory does not flush

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR 8185719 [testlibrary] : RMI TestSocketFactory does not flush

roger riggs
Please review a correction in the RMI TestSocketFactory
MatchReplaceOutputStream to correctly flush
partial matches.  It can cause hangs when the one or more bytes before a
flush are a partial match
for the pattern and the consumer is waiting for the pending bytes.

Webrev:
   http://cr.openjdk.java.net/~rriggs/webrev-flush-8185719/index.html

Issue:
   https://bugs.openjdk.java.net/browse/JDK-8185719

Thanks, Roger

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8185719 [testlibrary] : RMI TestSocketFactory does not flush

Lance Andersen
Hi Roger,

the changes seem reasonable.

Best
Lance

> On Aug 2, 2017, at 10:21 AM, Roger Riggs <[hidden email]> wrote:
>
> Please review a correction in the RMI TestSocketFactory MatchReplaceOutputStream to correctly flush
> partial matches.  It can cause hangs when the one or more bytes before a flush are a partial match
> for the pattern and the consumer is waiting for the pending bytes.
>
> Webrev:
>  http://cr.openjdk.java.net/~rriggs/webrev-flush-8185719/index.html
>
> Issue:
>  https://bugs.openjdk.java.net/browse/JDK-8185719
>
> Thanks, Roger
>

 <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
[hidden email] <mailto:[hidden email]>



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8185719 [testlibrary] : RMI TestSocketFactory does not flush

Daniel Fuchs
In reply to this post by roger riggs
Hi Roger,

I wonder if the MatchReplaceOutputStream should store the
partial match index at the time the partial bytes are flushed
in order to verify that this was indeed a partial match,
and report an error if it later finds that flush() was indeed
called during a full match? The potential Error to throw could
be created and recorded in flush() in order to diagnose
the offending flush() call...

On the other hand to do this properly you'd probably need
to maintain a stack of partial match indexes and corresponding
potential Errors to throw in case flush() is called again with
a partial match while the previous partial matching hasn't
been invalidated yet...

I mean - if your matched bytes are stg like  0 0 0 6
and the stream looks like:

   3 0 0 <flush> 0 <flush> ...

then either 6 or 0 0 6 coming after  the second flush would
make the test fail to find & replace the full match...

But maybe it's not worth it if you are sure that flush() cannot
be called during a full match/replace sequence?

best regards,

-- daniel



On 02/08/2017 15:21, Roger Riggs wrote:

> Please review a correction in the RMI TestSocketFactory
> MatchReplaceOutputStream to correctly flush
> partial matches.  It can cause hangs when the one or more bytes before a
> flush are a partial match
> for the pattern and the consumer is waiting for the pending bytes.
>
> Webrev:
>    http://cr.openjdk.java.net/~rriggs/webrev-flush-8185719/index.html
>
> Issue:
>    https://bugs.openjdk.java.net/browse/JDK-8185719
>
> Thanks, Roger
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8185719 [testlibrary] : RMI TestSocketFactory does not flush

roger riggs
Hi Daniel,

I considered keeping track of the partial match across the flush, but it
could only work
if the partial match was a prefix of the replacement string.   It got
complicated quickly.

Since this is used (only) for RMI, in which flushes are used at the end
of messages, it seemed
sufficient to document it as not working across flushes and with a debug
message
if flush was in the middle of a partial match.

Thanks, Roger


On 8/2/2017 11:25 AM, Daniel Fuchs wrote:

> Hi Roger,
>
> I wonder if the MatchReplaceOutputStream should store the
> partial match index at the time the partial bytes are flushed
> in order to verify that this was indeed a partial match,
> and report an error if it later finds that flush() was indeed
> called during a full match? The potential Error to throw could
> be created and recorded in flush() in order to diagnose
> the offending flush() call...
>
> On the other hand to do this properly you'd probably need
> to maintain a stack of partial match indexes and corresponding
> potential Errors to throw in case flush() is called again with
> a partial match while the previous partial matching hasn't
> been invalidated yet...
>
> I mean - if your matched bytes are stg like  0 0 0 6
> and the stream looks like:
>
>   3 0 0 <flush> 0 <flush> ...
>
> then either 6 or 0 0 6 coming after  the second flush would
> make the test fail to find & replace the full match...
>
> But maybe it's not worth it if you are sure that flush() cannot
> be called during a full match/replace sequence?
>
> best regards,
>
> -- daniel
>
>
>
> On 02/08/2017 15:21, Roger Riggs wrote:
>> Please review a correction in the RMI TestSocketFactory
>> MatchReplaceOutputStream to correctly flush
>> partial matches.  It can cause hangs when the one or more bytes
>> before a flush are a partial match
>> for the pattern and the consumer is waiting for the pending bytes.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~rriggs/webrev-flush-8185719/index.html
>>
>> Issue:
>>    https://bugs.openjdk.java.net/browse/JDK-8185719
>>
>> Thanks, Roger
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8185719 [testlibrary] : RMI TestSocketFactory does not flush

Daniel Fuchs
Hi Roger,

On 02/08/2017 16:32, Roger Riggs wrote:

> Hi Daniel,
>
> I considered keeping track of the partial match across the flush, but it
> could only work
> if the partial match was a prefix of the replacement string.   It got
> complicated quickly.
>
> Since this is used (only) for RMI, in which flushes are used at the end
> of messages, it seemed
> sufficient to document it as not working across flushes and with a debug
> message
> if flush was in the middle of a partial match.

OK, looks good then.
It occurred to me that the current match detection is eager anyway.
That is, if the matchBytes are { 0, 0, 0, 6 } and the stream
is { 3 0 0 0 0 6 0 ... } then unless I'm mistaken no match will
be detected, because matching will start at byte 1 (it would
need to start at byte 2 to detect a full match).

So I guess this is a best effort which happens to work correctly
with the input we use in our tests?

best regards,

-- daniel

>
> Thanks, Roger
>
>
> On 8/2/2017 11:25 AM, Daniel Fuchs wrote:
>> Hi Roger,
>>
>> I wonder if the MatchReplaceOutputStream should store the
>> partial match index at the time the partial bytes are flushed
>> in order to verify that this was indeed a partial match,
>> and report an error if it later finds that flush() was indeed
>> called during a full match? The potential Error to throw could
>> be created and recorded in flush() in order to diagnose
>> the offending flush() call...
>>
>> On the other hand to do this properly you'd probably need
>> to maintain a stack of partial match indexes and corresponding
>> potential Errors to throw in case flush() is called again with
>> a partial match while the previous partial matching hasn't
>> been invalidated yet...
>>
>> I mean - if your matched bytes are stg like  0 0 0 6
>> and the stream looks like:
>>
>>   3 0 0 <flush> 0 <flush> ...
>>
>> then either 6 or 0 0 6 coming after  the second flush would
>> make the test fail to find & replace the full match...
>>
>> But maybe it's not worth it if you are sure that flush() cannot
>> be called during a full match/replace sequence?
>>
>> best regards,
>>
>> -- daniel
>>
>>
>>
>> On 02/08/2017 15:21, Roger Riggs wrote:
>>> Please review a correction in the RMI TestSocketFactory
>>> MatchReplaceOutputStream to correctly flush
>>> partial matches.  It can cause hangs when the one or more bytes
>>> before a flush are a partial match
>>> for the pattern and the consumer is waiting for the pending bytes.
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~rriggs/webrev-flush-8185719/index.html
>>>
>>> Issue:
>>> https://bugs.openjdk.java.net/browse/JDK-8185719
>>>
>>> Thanks, Roger
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8185719 [testlibrary] : RMI TestSocketFactory does not flush

roger riggs
Hi Daniel,

Yep, it does not try/need to handle the case where a prefix of the match
appears within the match.
When a mismatch is detected, it starts fresh for a match. Someday, it
may need an upgrade,
but for now its sufficient.

Thanks, Roger


On 8/2/2017 11:43 AM, Daniel Fuchs wrote:

> Hi Roger,
>
> On 02/08/2017 16:32, Roger Riggs wrote:
>> Hi Daniel,
>>
>> I considered keeping track of the partial match across the flush, but
>> it could only work
>> if the partial match was a prefix of the replacement string. It got
>> complicated quickly.
>>
>> Since this is used (only) for RMI, in which flushes are used at the
>> end of messages, it seemed
>> sufficient to document it as not working across flushes and with a
>> debug message
>> if flush was in the middle of a partial match.
>
> OK, looks good then.
> It occurred to me that the current match detection is eager anyway.
> That is, if the matchBytes are { 0, 0, 0, 6 } and the stream
> is { 3 0 0 0 0 6 0 ... } then unless I'm mistaken no match will
> be detected, because matching will start at byte 1 (it would
> need to start at byte 2 to detect a full match).
>
> So I guess this is a best effort which happens to work correctly
> with the input we use in our tests?
>
> best regards,
>
> -- daniel
>
>>
>> Thanks, Roger
>>
>>
>> On 8/2/2017 11:25 AM, Daniel Fuchs wrote:
>>> Hi Roger,
>>>
>>> I wonder if the MatchReplaceOutputStream should store the
>>> partial match index at the time the partial bytes are flushed
>>> in order to verify that this was indeed a partial match,
>>> and report an error if it later finds that flush() was indeed
>>> called during a full match? The potential Error to throw could
>>> be created and recorded in flush() in order to diagnose
>>> the offending flush() call...
>>>
>>> On the other hand to do this properly you'd probably need
>>> to maintain a stack of partial match indexes and corresponding
>>> potential Errors to throw in case flush() is called again with
>>> a partial match while the previous partial matching hasn't
>>> been invalidated yet...
>>>
>>> I mean - if your matched bytes are stg like  0 0 0 6
>>> and the stream looks like:
>>>
>>>   3 0 0 <flush> 0 <flush> ...
>>>
>>> then either 6 or 0 0 6 coming after  the second flush would
>>> make the test fail to find & replace the full match...
>>>
>>> But maybe it's not worth it if you are sure that flush() cannot
>>> be called during a full match/replace sequence?
>>>
>>> best regards,
>>>
>>> -- daniel
>>>
>>>
>>>
>>> On 02/08/2017 15:21, Roger Riggs wrote:
>>>> Please review a correction in the RMI TestSocketFactory
>>>> MatchReplaceOutputStream to correctly flush
>>>> partial matches.  It can cause hangs when the one or more bytes
>>>> before a flush are a partial match
>>>> for the pattern and the consumer is waiting for the pending bytes.
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~rriggs/webrev-flush-8185719/index.html
>>>>
>>>> Issue:
>>>> https://bugs.openjdk.java.net/browse/JDK-8185719
>>>>
>>>> Thanks, Roger
>>>>
>>>
>>
>

Loading...