RFR(s): 8148425: strerror() function is not thread-safe
Coleen Phillimore
coleen.phillimore at oracle.com
Tue Mar 8 22:01:17 UTC 2016
This latest version seems okay (or at least better than the other
workarounds).
thanks,
Coleen
On 3/1/16 12:30 AM, David Holmes wrote:
> Hi Thomas,
>
> On 1/03/2016 3:23 AM, Thomas Stüfe wrote:
>> Hi David,
>>
>> On Mon, Feb 29, 2016 at 4:44 AM, David Holmes <david.holmes at oracle.com
>> <mailto:david.holmes at oracle.com>> wrote:
>>
>> Hi Thomas,
>>
>> On 27/02/2016 2:05 AM, Thomas Stüfe wrote:
>>
>> Hi,
>>
>> please take a look at this proposed fix:
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8148425
>> Webrev:
>> http://cr.openjdk.java.net/~stuefe/webrevs/8141425-strerror-replacement/webrev.00/webrev/
>>
>> This adds a replacement function os::strerror() as a drop-in for
>> strerror(), which has a number of issues.
>>
>> strerror() is unsafe to use and may cause (and has caused)
>> crashes in
>> multithreaded scenarios. It is also not ideal for log files
>> because of the
>> implicit localization of the error messages.
>>
>> For details please see the discussion under the bug report.
>>
>>
>> I just came across strerror_l, which is required to be thread-safe.
>> Is that a possible alternative? (I'm not sure how locale's are
>> obtained).
>>
>>
>> Sorry, I think this API is glibc only. At least I cannot find this in
>> our AIX headers, nor on Solaris.
>
> It is a POSIX API:
>
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/strerror_l.html
>
> added in 2006 but part of POSIX.1-2008. But as per Dalibor's link not
> necessarily available:
>
> "This function is missing on some platforms: glibc 2.3.6, Mac OS X
> 10.5, FreeBSD 6.0, NetBSD 5.0, OpenBSD 3.8, Minix 3.1.8, AIX 5.1,
> HP-UX 11, IRIX 6.5, OSF/1 5.1, Solaris 11 2011-11, Cygwin, mingw, MSVC
> 9, Interix 3.5, BeOS. "
>
> Pity.
>
>
>> Otherwise what you have seems okay - though I do dislike having to
>> duplicate all that details already buried in the system
>> headers/library. Not sure we need to the long text at the VM level -
>> which would simplify things a bit.
>>
>>
>> I agree, I dislike this too. Like everyone else in this thread. But I
>> think this is a pragmatic solution.
>>
>> I am a bit stuck here - should we really get rid of the long text
>> feature? There are some callsites of strerror() in the hotspot where
>> arguably the long text is better suited:
>> - in assert_status() (see debug.hpp) - this ends up in the header of
>> error files, if this suddenly changes to a literalized errno, people may
>> be upset
>
> I added assert_status purely to expand on what the failing status was
> :) I'm happy to see EINVAL in that context rather than "invalid
> argument" (or whatever) :)
>
>> - when failing to write a heap dump file - see services/heapDumper.cpp.
>> Which ends as printout on the command line, I think.
>
> Maybe ... if it is an error the user can fix.
>
>> The safe option would be to provide both variants (short and long text).
>> Or, provide the safe, short variant for all logging calls - when
>> "EINVAL" is enough, and let users continue to use strerror() for those
>> few cases where the long text is needed.
>>
>> What do you think?
>
> Trying to decide what makes me least unhappy. :) Given you've already
> done the work I guess having long and short forms is okay.
>
> Thanks,
> David
>
>> Thanks, Thomas
>>
>> Thanks,
>> David
>>
>>
>> Please note that I did not yet change any call sites, although
>> all call
>> sites in the os namespace should already use the new function. I
>> wanted to
>> see whether there would be any general objections.
>>
>> Kind Regards, Thomas
>>
>>
More information about the hotspot-runtime-dev
mailing list