iconv_set_encoding intermittently crashing

[chrisgraham]

Running hphpi I logged all output over a period of time on a live site, and got a lot of intermittent segmentation faults all from iconv_set_encoding. This is the only segmentation fault I got actually, but I got it about 15 times over a few hours. It happens on normal page requests, from different normal entry URLs, and I have double checked to ensure there is nothing special about those cases.

Core dumped: Segmentation fault
    #0 iconv_set_encoding(), called at [/home/ocp/public_html/sources/global2.php:1868]
    #1 convert_data_encodings(), called at [/home/ocp/public_html/sources/global2.php:358]
    #2 init__global2(), called at [:0]
    #3 call_user_func(), called at [/home/ocp/public_html/sources/global.php:171]
    #4 require_code(), called at [/home/ocp/public_html/sources/global.php:420]
    #5 include(), called at [/home/ocp/public_html/backend.php:44]

The actual call will be:

iconv_set_encoding('input_encoding','ISO-8859-1');

[scottmac]

Can you build a debug build of hphp, it makes things slightly slower but I need more information. A lot of your stacktraces while useful don't hint at what the exact problem is.

Looking at the code its pretty straight forward, update a static class variable with the current value.

bool f_iconv_set_encoding(CStrRef type, CStrRef charset) {
  if (!validate_charset(charset)) return false;
  if (type == "input_encoding") {
    ICONVG(input_encoding) = charset;
  } else if (type == "output_encoding") {
    ICONVG(output_encoding) = charset;
  } else if (type == "internal_encoding") {
    ICONVG(internal_encoding) = charset;
  } else {
    return false;
  }
  return true;
}

[chrisgraham]

Yes, I'll set up a debug build and hopefully get back to you.

However one thing... hphp[i] does not include date's when it spits out errors. Any chance of a patch that'll do that before I do it, so I can correlate my core dumps against the logged times?

[scottmac]

Start with the following and it should log more information including the date.

-v "Log.Header=on" -v "Log.InjectedStackTrace=on"

[chrisgraham]

Actually I'll just dump the date out to my log in the relaunch loop I had to write when I relaunch. However, this'd still be helpful in the future so people can correlate non-crashing errors against other logs.

[chrisgraham]

ah - you beat me - thanks

[chrisgraham]

I think you mean =true rather than =on, or do both work?

[scottmac]

true, On, 1 all work here.

[chrisgraham]

#0  0x0000003169c79d1c in free () from /lib64/libc.so.6
#1  0x0000000005a62567 in HPHP::StringData::release() ()
#2  0x00000000059ee32a in HPHP::String::operator=(char const*) ()
#3  0x0000000005b4504c in HPHP::ICONVGlobals::requestInit() ()
#4  0x00007facbe1f5cd0 in ?? ()
#5  0x00007facbe1f55b0 in ?? ()
#6  0x0000000005b420c0 in HPHP::f_iconv_set_encoding(HPHP::String const&, HPHP::String const&) ()

I think we will find consistently all my crashes are to do with memory management.

[chrisgraham]

#0  0x0000003169c326c5 in raise () from /lib64/libc.so.6
#1  0x0000003169c33ea5 in abort () from /lib64/libc.so.6
#2  0x0000003169c6f133 in __libc_message () from /lib64/libc.so.6
#3  0x0000003169c74ac6 in malloc_printerr () from /lib64/libc.so.6
#4  0x0000000005a62567 in HPHP::StringData::release() ()
#5  0x00000000059ee32a in HPHP::String::operator=(char const*) ()
#6  0x0000000005b4504c in HPHP::ICONVGlobals::requestInit() ()
#7  0x00007f56d57f4f30 in ?? ()
#8  0x00007f56d57f4810 in ?? ()
#9  0x0000000005b420c0 in HPHP::f_iconv_set_encoding(HPHP::String const&, HPHP::String const&) ()
#10 0x00007f56d57f4fe0 in ?? ()
#11 0x000017ffd57f4f01 in ?? ()
#12 0x000000000957ada1 in ?? ()
#13 0x000000000957ada0 in ?? ()
#14 0x000000000000000a in ?? ()
#15 0x0000000000000001 in ?? ()
#16 0x00000000059daf4e in HPHP::is_numeric_string(char const*, int, long long*, double*, int) ()

[scottmac]

For your iconv_set_encoding() calls, is it straight string values or are they coming form APC?

Copying that line / block of code would be great. Also are you running the interpeter or compiled?

[chrisgraham]

function get_charset()
{
    if (function_exists('do_lang'))
    {
        $attempt=do_lang('charset',NULL,NULL,NULL,NULL,false);
        if (!is_null($attempt)) return $attempt;
    }
    
    global $SITE_INFO;
    $lang=array_key_exists('default_lang',$SITE_INFO)?$SITE_INFO['default_lang']:'EN';
    $path=get_file_base().'/lang_custom/'.$lang.'/global.ini';
    if (!file_exists($path)) $path=get_file_base().'/lang/'.$lang.'/global.ini';
    if (!file_exists($path))
    {
        $path=get_file_base().'/lang_custom/'.$lang.'/global.po';
        if (file_exists($path)) return 'utf-8';
    }
    if (!file_exists($path))
    {
        $path=get_file_base().'/lang_custom/'.$lang.'/global-'.strtolower($lang).'.po';
        if (file_exists($path)) return 'utf-8';
    }
    if (!file_exists($path)) $path=get_file_base().'/lang/EN/global.ini';
    $file=fopen($path,'rt');
    $contents=str_replace(chr(13),chr(10),fread($file,100));
    fclose($file);
    $matches=array();
    if (preg_match('#charset=([\w\-]+)\n#',$contents,$matches)!=0)
    {
        return strtolower($matches[1]);
    }
    return strtolower('ISO-8859-1');
}

    $charset=get_charset();

        $encoding=$known_utf8?'UTF-8':$charset;
        if (@iconv_set_encoding('input_encoding',$encoding))

Computed strings. I've put together the main involved code above, but I don't think it's particularly relevant. The string causing the crash probably comes from a manually parsed ini file, or an entry from a de-serialized array.

[chrisgraham]

Happens on both hphpi and compiled.

I think it's very likely all my problems are related to some kind of memory error. My codebase is huge, so perhaps it's calling some function in the implementation that is corrupting memory. Now I'm seeing occasional garbled strings, like DB queries failing as they've been put together with corrupt data or out of order. Also I posted before about an issue with global variables getting unset.

If you like I can send you my executable, core dumps, and source code. It's going to be very difficult for me to try and narrow down on an intermittent problem and I do't have the time to be honest.

[chrisgraham]

It seems github had an issue there and lost one of your replies and one of mine Scott. Hopefully you got my last reply before the one that shows.

[chrisgraham]

I've had a look at the HPHP code. I'm not that good at C++ to fully understand it (never worked with templates), but the gist I get is that String is not correctly initialising it's data pointer on my system.

So for iconv the issue starts very simply - it is setting a string, which should have been initialised with an internal m_px of NULL.

class ICONVGlobals : public RequestEventHandler {
public:
  String input_encoding;
  String output_encoding;
  String internal_encoding;

  ICONVGlobals() {}

  virtual void requestInit() {
    input_encoding = "ISO-8859-1";
    output_encoding = "ISO-8859-1";
    internal_encoding = "ISO-8859-1";
  }

  virtual void requestShutdown() {
  }
};

However the SmartPtr code is not seeing that, and trying to do an object delete on what I presume is an arbitrary junk pointer.

  template
  SmartPtr &operator=(Y *px) {
    if (m_px != px) {
      if (m_px && m_px->decRefCount() == 0) {
        m_px->release();
      }
      m_px = dynamic_cast(px);
      if (m_px) m_px->incRefCount();
    }
    return *this;
  }

As surely if things are working right m_px is going to be NULL for the initialisation, and hence 'release' not called.

Anyway that's what I figure. I don't know templates enough to try and work out how m_px is supposed to be initialised and I certainly haven't tried to step through the code or study in depth.

[chrisgraham]

In type_string.h we have:

  String() {}

so would this (and disclaimer again about me not knowing C++ very well) override and not chain the initialiser,

  SmartPtr() : m_px(NULL) {}

hence leaving m_px uninitialised?