php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #59191 persistent connection + multiple calls per child = segfault
Submitted: 2010-05-05 11:30 UTC Modified: 2012-03-07 17:32 UTC
Votes:2
Avg. Score:3.0 ± 0.0
Reproduced:0 of 0 (0.0%)
From: abodera at gmail dot com Assigned:
Status: Analyzed Package: memcached (PECL)
PHP Version: 5.3.2 OS: Fedora 12 / Gentoo / Mac
Private report: No CVE-ID: None
View Add Comment Developer Edit
Anyone can comment on a bug. Have a simpler test case? Does it work for you on a different platform? Let us know!
Just going to say 'Me too!'? Don't clutter the database with that please — but make sure to vote on the bug!
Your email address:
MUST BE VALID
Solve the problem:
38 - 5 = ?
Subscribe to this entry?

 
 [2010-05-05 11:30 UTC] abodera at gmail dot com
Description:
------------
Running the following piece of code on 5.3.2 will cause a segfault under CLI. It will also kill an apache child with "zend_mm_heap corrupted" written into error_log.

Of course this code should be avoided (there is no real need for such double instantiation of the same connection), but it happened in my application and caused a lot of trouble before pinpointed.

The problem does NOT occur with the same SVN version of memcached + PHP 5.2.9, so I'd say it's >5.3 specific. Also, it's probably easy to solve, as multiple calls with the same connection id should return the same resource.

Also: Thanks for great extension! :)

A.

Reproduce code:
---------------
<?php
error_reporting(E_ALL);
ini_set('display_errors',1);

$m = new Memcached('test');
if(!count($m->getServerList())){
        $m->addServer('127.0.0.101',11211);
        $m->addServer('127.0.0.102',11211);
}


$m = new Memcached('test');
if(!count($m->getServerList())){
        $m->addServer('127.0.0.101',11211);
        $m->addServer('127.0.0.102',11211);
}


Actual result:
--------------
CLI: segmentation fault
apache2: zend_mm_heap corrupted



Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2010-05-05 11:31 UTC] abodera at gmail dot com
.
 [2010-05-06 02:16 UTC] brianm at dealnews dot com
Similar issue with persistent connecitons on PHP 5.2.13 Linux and Mac 5.3.1.

http://pastebin.com/40vzVM7D

As you can see, pecl/memcache does not show the same behavior.

Not exactly the same. But, I don't want to file a second bug in case it is the same issue.

In my case, I see this connection growth over time in Apache with normal use where more than one object is not made per request.
 [2010-05-06 02:18 UTC] andrei@php.net
Any way you can test it with the master branch from github and see if it still happens?
 [2010-05-06 11:15 UTC] Jared dot Williams at ntlworld dot com
No problems here

jared@ubuntu:~$ cat bug.php
<?php
error_reporting(E_ALL);
ini_set('display_errors',1);

$m = new Memcached('test');
if(!count($m->getServerList())){
        $m->addServer('127.0.0.101',11211);
        $m->addServer('127.0.0.102',11211);
}

$m = new Memcached('test');
if(!count($m->getServerList())){
        $m->addServer('127.0.0.101',11211);
        $m->addServer('127.0.0.102',11211);
}
jared@ubuntu:~$ php bug.php

jared@ubuntu:~$ php -v
PHP 5.3.2 (cli) (built: May  3 2010 17:24:22)
Copyright (c) 1997-2010 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2010 Zend 
Technologies
jared@ubuntu:~$ cd Desktop/php-memcached/ && git rev-parse 
HEAD && cd ../..
19b048921d6ac15e83cfd8c091e5d0a43c4078d8
jared@ubuntu:~$ uname -a
Linux ubuntu 2.6.32-21-generic #32-Ubuntu SMP Fri Apr 16 
08:09:38 UTC 2010 x86_64 
GNU/Linux
 [2010-05-06 11:53 UTC] andrei@php.net
I'm afraid you'll have to wait for 2.0 release. I'm working on wrapping it up and hope to release the beta shortly.
 [2010-05-07 11:23 UTC] abodera at gmail dot com
Please don't close it. It's a blocker.
There are more segfaults even without the code below. I'm trying to track it down.
 [2010-05-07 12:28 UTC] andrei@php.net
Brian, this (connection growing issue) was fixed in 2.0.0-dev code which is in master branch on Github. It will be part of 2.0.0 release.
 [2010-06-28 17:04 UTC] brianm at dealnews dot com
I am still seeing this connection issue with the code from GitHub.

# php -i | fgrep -C 2 "memcached support"
memcached

memcached support => enabled
Version => 2.0.0-dev
libmemcached version => 0.39


<?php

$pid = getmypid();

for($x=0;$x<20;$x++) {

    $mc = new Memcached("test");
    $mc->addServer("localhost", 11211);
    $mc->get("foo");
    unset($mc);

    $connections = trim(`lsof -n -p $pid | fgrep -c "11211"`);
    $mem = trim(`fgrep VmRSS /proc/$pid/status`);

    echo "$x: $connections conns, $mem\n";
}

?>

This outputs:

# php memcached.php 
0: 1 conns, VmRSS:	   10400 kB
1: 2 conns, VmRSS:	   10420 kB
2: 2 conns, VmRSS:	   10424 kB
3: 2 conns, VmRSS:	   10432 kB
4: 2 conns, VmRSS:	   10436 kB
5: 3 conns, VmRSS:	   10444 kB
6: 3 conns, VmRSS:	   10452 kB
7: 4 conns, VmRSS:	   10464 kB
8: 4 conns, VmRSS:	   10468 kB
9: 4 conns, VmRSS:	   10472 kB
10: 5 conns, VmRSS:	   10484 kB
11: 6 conns, VmRSS:	   10492 kB
12: 6 conns, VmRSS:	   10496 kB
13: 6 conns, VmRSS:	   10504 kB
14: 6 conns, VmRSS:	   10504 kB
15: 7 conns, VmRSS:	   10512 kB
16: 7 conns, VmRSS:	   10516 kB
17: 7 conns, VmRSS:	   10524 kB
18: 8 conns, VmRSS:	   10532 kB
19: 8 conns, VmRSS:	   10536 kB
 [2010-08-05 21:17 UTC] brianm at dealnews dot com
Andrei, I have become aware of a work around for this issue by checking if there is a server list before adding servers.

1. Was this your intended behavior?
2. If so, can you document it as such?

If this was the intended behavior, I can certainly roll with that. But, your stance so far on this bug leads me to believe it was not.
 [2010-08-21 17:41 UTC] brianm at dealnews dot com
Based on what is in github, it looks like the user land best practice is this:

$servers = array(
    array(
        "host" => "localhost",
        "port" => 11211,
        "weight" => 1
    ),
    array(
        "host" => "localhost",
        "port" => 11211,
        "weight" => 2
    ),
    array(
        "host" => "localhost",
        "port" => 11211,
        "weight" => 3
    ),
);

$mc = new Memcached("test");
if($mc->isPristine()){
    $server_list = $mc->getServerList();
    if($server_list != $servers){
        $server_keys = array();
        foreach($server_list as $sl){
            $key = $sl["host"].':'.$sl["port"].':'.$sl["weight"];
            $server_keys[$key] = true;
        }
        foreach($servers as $s){
            $key = $s["host"].':'.$s["port"].':'.$s["weight"];
            if(!isset($server_keys[$key])){
                $mc->addServer($s["host"], $s["port"], $s["weight"]);
            }
        }
    }
}

OR:

$mc = new Memcached("test");
if($mc->isPristine()){
    foreach($servers as $s){
        $mc->addServer($s["host"], $s["port"], $s["weight"]);
    }
}

The first method could lead to server lists varying across nodes if code roll out is not handled well to different servers. But, I am wondering if we can just get one of these two logics added to the addServer and addServers methods and not have to do it in user space.
 [2010-08-21 17:52 UTC] brianm at dealnews dot com
In fact, here is a patch for the simplest of the methods.

diff --git a/php_memcached.c b/php_memcached.c
index 1323d5e..3072817 100644
--- a/php_memcached.c
+++ b/php_memcached.c
@@ -1619,6 +1619,11 @@ PHP_METHOD(Memcached, addServer)
        MEMC_METHOD_FETCH_OBJECT;
        i_obj->rescode = MEMCACHED_SUCCESS;
 
+       if(!i_obj->is_pristine) {
+               RETURN_TRUE;
+       }
+
+
        if (host[0] == '/') { /* unix domain socket */
                status = memcached_server_add_unix_socket_with_weight(m_obj->memc, host, weight);
        } else {
@@ -1653,6 +1658,10 @@ PHP_METHOD(Memcached, addServers)
        MEMC_METHOD_FETCH_OBJECT;
        i_obj->rescode = MEMCACHED_SUCCESS;
 
+       if(!i_obj->is_pristine) {
+               RETURN_TRUE;
+       }
+
        for (zend_hash_internal_pointer_reset(Z_ARRVAL_P(servers)), i = 0;
                 zend_hash_get_current_data(Z_ARRVAL_P(servers), (void **)&entry) == SUCCESS;
                 zend_hash_move_forward(Z_ARRVAL_P(servers)), i++) {
 [2011-08-18 02:30 UTC] drlung at pacbell dot net
There's a sceert about your post. ICTYBTIHTKY
 [2011-08-30 21:39 UTC] h-yamano at nirai dot ne dot jp
<a href="http://www.pillscity.net/">valtrex</a> 6104 <a href="http://www.medmuster.com/">priligy</a> xadky
 [2011-09-03 21:42 UTC] weekly at goya dot co dot jp
<a href="http://www.pillscity.net/">buy valtrex here</a> 440 <a href="http://www.topmdhelp.com/">topamax</a> >:(
 [2011-09-14 20:07 UTC] meikou at beach dot ocn dot ne dot jp
<a href="http://www.halfpricemed.com/">buy viagra cheap</a> 5216 <a href="http://www.topratedpills.net/">topamax+wellbutrin</a> wdwqpu
 [2012-03-07 17:32 UTC] andrei@php.net
-Status: Open +Status: Analyzed
 [2017-08-15 17:55 UTC] php at haravikk dot me
Was this issue ever resolved? Does persistence now work as expected?

I was hoping to use the libketama compatible consistent distribution mode, but if persistence is still broken then I'm concerned that the overhead of creating the "continuum" would be too high without persistence.
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Mon Nov 11 22:01:32 2019 UTC