Sean,
The fix looks good for me but the code might be better readable if you
inverse the condition.
if (!(s instanceof PlainSocketImpl)) {
impl.accept(s)
return;
}
... rest of the code
-Dmitry
On 2013-10-01 18:54, Daniel Fuchs wrote:
> On 10/1/13 4:50 PM, Seán Coffey wrote:
>>
>> On 01/10/2013 14:51, Daniel Fuchs wrote:
>>> On 10/1/13 3:09 PM, Seán Coffey wrote:
>>>> Since I'm only creating a dummy socketImpl to test the
>>>> classcastexception, no real networking stack is in place here. I'm
>>>> catching the NPE that would be thrown from the native
>>>> Java_java_net_TwoStacksPlainSocketImpl_socketAccept function since the
>>>> underlying socket passed to it is null.
>>>>
>>>> C:\tmp>java CustomSocketImplFactory
>>>> Created Socket[addr=null,port=0,localport=0]
>>>> Exception in thread "main" java.lang.NullPointerException: socket is
>>>> null
>>>> at java.net.TwoStacksPlainSocketImpl.socketAccept(Native
>>>> Method)
>>>
>>> That's what I would have expected from your previous changeset.
>>>
>>> But you're no longer passing null - right? Now you're passing
>>> an instance of CustomSocketImpl.
>>>
>>> So where does the NPE come from? Could it be because you should
>>> be calling:
>>> ServerSocket.setSocketImplFactory(new CustomSocketImplFactory());
>>> and not:
>>> Socket.setSocketImplFactory(new CustomSocketImplFactory()); ?
>> The original bug stemmed from a custom socket Impl interacting with the
>> JDK ServerSocket Impl. If I move both Socket and ServerSocket factory
>> implementations over to use the custom Impl, the testcase doesn't get to
>> walk through the JDK serverSocket code and the ClassCastException is not
>> seen.
>>
>> The NPE seen is coming from down in the native stack when the JDK
>> ServerSocket is running through accept request (from our client socket).
>> I don't think it's an issue for this case.
>>
>> line 611 :
>> http://hg.openjdk.java.net/jdk8/tl/jdk/file/tip/src/windows/native/java/net/TwoStacksPlainSocketImpl.c
>>
>>
>>
>> regards,
>> Sean.
>
> Thanks Seán. You convinced me.
>
> -- daniel
>
>>> Or should you call both?
>>>
>>> best regards,
>>>
>>> -- daniel
>>>
>>>
>>>>
>>>> Regards,
>>>> Sean.
>>>>
>>>>>
>>>>> Or is that going to hide future bugs?
>>>>>
>>>>> best regards
>>>>>
>>>>> -- daniel (not a reviewer)
>>>>
>>>
>>
>
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.
