RFR(S): 8010190: os_windows missing winsock initialization

[Nils Eliasson]

Hi David,

Thanks for your feedback!

The lazy sockInit fix is not as ad-hoc as it might look - on all other 
paths a file descriptor is required, i.e. one of the paths with a call 
to initSock has to be called first.

Anyway I took a look at 7034299 and have addressed the race. In the new 
webrev I am removing all the synchronization code and lazy 
initialization, and calling initSock from os::init_2 when 
PrintIdealGraphLevel is used. WinSock will only be initialized when 
needed, and it will be done from single threaded code. If more users of 
WinSock is added later, then we should consider always calling initSock.

I choose not to improve the failure handling. IdealGraphVisualizer is 
currently the only consumer of WinSockAPI (through 
ostream/networkstream) and it is only available in debug/fastdebug. We 
hit asserts if the init has not been run or if the networkstream can't 
connect to the client.

New webrev:
http://cr.openjdk.java.net/~neliasso/7034299/webrev.02/

Old bug:
http://http://bugs.sun.com/view_bug.do?bug_id=7034299

Thanks,
Nils Eliasson

On 2013-03-19 01:51, David Holmes wrote:
> Hi Nils,
>
> You fix is fine as far as it goes but ...
>
> ... basically if we make changes as to how this code gets used we will 
> hit new failures until slowly we add an initialization check to all of 
> the methods. This is horribly ad-hoc. Worse there is a race condition 
> (a couple actually) in the initialization code which we are more 
> likely to hit now that we have more paths to this code.
>
> Seems to me that the real fix for this code might be to ensure 
> sockInit is called during VM startup instead of lazily? Otherwise, as 
> I said, there's a bunch of race conditions that need fixing anyway.
>
> Cheers,
> David
>
> On 18/03/2013 9:57 PM, Nils Eliasson wrote:
>> Hi all,
>>
>> A small change that adds lazy initalization of winsock (os::initsock())
>> on some paths that needs it. WinSock has only a few uses (streaming
>> graphs to IdealGraphVisualizers a.f.a.i.k.) so the paths are not hot,
>> and the initialization will not add any overhead to regular VM use.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~neliasso/8010190/webrev.01/
>>
>> Bug:
>> http://bugs.sun.com/view_bug.do?bug_id=80010190
>>
>> Thanks,
>> Nils Eliasson