[jira] [Commented] (MESOS-3551) Replace use of strerror with thread-safe alternatives strerror_r / strerror_l.

["Benjamin Bannier (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/MESOS-3551?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14946733#comment-14946733 ] 

Benjamin Bannier commented on MESOS-3551:
-----------------------------------------

OK, the 0th implementation brought some open ends to the surface.

h5. strerror_r vs. strerror_l

To not break OS X compatibility {{strerror_l}} seems out of reach and we need to resort to {{strerror_r}}.

h5. Glibc provides a non-compliant and potentially broken {{strerror_r}}

Since {{strerror}} uses are currently all across stout (a few), libprocess (a handful), and mesos (plenty), a natural place to implement a wrapper should probably be in stout.

Since stout is header-only a reusable wrapper implementation would probably under the covers use any available implementation (or: _Should this be a reason to (a) implement the wrapper higher up, e.g. in libprocess, or (b) make stout include compiled components, or (c) no, leave it in stout?_).


Assuming we decide to implement this wrapper in a header we would also decide on how to deal with a bug in glibc-2.15:

bq. https://sourceware.org/bugzilla/show_bug.cgi?id=12204

Here glibc's {{strerror_r}} might set the global {{errno}} should it run into errors itself (e.g. because the passed errnum was invalid) which is not compliant and probably unexpected. Fixed versions where shipped e.g. starting with Debian8, CentOS7, Ubuntu14.04.

Since the mesos {{configure.ac}} already requires at least gcc-4.8.0 which is not satisfied by stock Debian7 (gcc-4.7.2), CentOS6 (gcc-4.4.7), or Ubuntu12.04 (gcc-4.6.3) _we could provide an implementation for either (a) only glibc-2.16+, or (b) introduce a workaround if we are using an old version_. If we decide on (a) it appears that adding a configure check wouldn't be sufficient to prevent someone from including the header from stout so we would need to add checks and potentially {{#errors}} in the implementation itself.


h5. Localized error messages

We cannot know the maximal length of error messages since they might be localized. We could either

  (a) implement an algorithm growing the buffer used by {{strerror_r}} until the message fits in, or
  (b) use a fixed buffer size with an educated guess about the maximal error message length (say 2000 char like used in {{llvm::sys::StrError}}).

Given the complexity workarounds for glibc non-conformance might introduce I feel option (b) might be good enough for now.


Any input welcome.


> Replace use of strerror with thread-safe alternatives strerror_r / strerror_l.
> ------------------------------------------------------------------------------
>
>                 Key: MESOS-3551
>                 URL: https://issues.apache.org/jira/browse/MESOS-3551
>             Project: Mesos
>          Issue Type: Bug
>          Components: libprocess, stout
>            Reporter: Benjamin Mahler
>            Assignee: Benjamin Bannier
>              Labels: newbie, tech-debt
>
> {{strerror()}} is not required to be thread safe by POSIX and is listed as unsafe on Linux:
> http://pubs.opengroup.org/onlinepubs/9699919799/
> http://man7.org/linux/man-pages/man3/strerror.3.html
> I don't believe we've seen any issues reported due to this. We should replace occurrences of strerror accordingly, possibly offering a wrapper in stout to simplify callsites.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)