Skip to content

fix MemcachedServer #474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Aug 23, 2021
Merged

Conversation

m6w6
Copy link
Contributor

@m6w6 m6w6 commented Jan 19, 2021

@remicollet
Copy link
Collaborator

remicollet commented Jan 19, 2021

Tried, and works as expected (using server-example scripts)

So +1

P.S. server-example/run-server.php + server-example/set-get.php is OK,
but server-example/run-server.php + server-example/test-server.php raise segfault.

@remicollet
Copy link
Collaborator

@m6w6 here is a backtracen seems related to flush call

(gdb) bt
#0  s_flush_handler (when=<optimized out>, cookie=<optimized out>) at /work/GIT/pecl-and-ext/memcached/php_memcached_server.c:304
#1  s_flush_handler (cookie=<optimized out>, when=<optimized out>) at /work/GIT/pecl-and-ext/memcached/php_memcached_server.c:291
#2  0x00007ffff7f95b20 in flush_command_handler () from /lib64/libmemcachedprotocol.so.0
#3  0x00007ffff7f96bdc in memcached_binary_protocol_process_data () from /lib64/libmemcachedprotocol.so.0
#4  0x00007ffff7f97a0e in memcached_protocol_client_work () from /lib64/libmemcachedprotocol.so.0
#5  0x00007ffff754be18 in s_handle_memcached_event (fd=7, what=<optimized out>, arg=0x7ffff726c120) at /work/GIT/pecl-and-ext/memcached/php_memcached_server.c:626
#6  0x00007ffff746c774 in event_once_cb () from /lib64/libevent-2.1.so.6
#7  0x00007ffff7471b54 in event_process_active_single_queue () from /lib64/libevent-2.1.so.6
#8  0x00007ffff74735d7 in event_base_loop () from /lib64/libevent-2.1.so.6
#9  0x00007ffff754df56 in php_memc_proto_handler_run (handler=0x7ffff7257a00, address=0x7ffff7294cf8) at /work/GIT/pecl-and-ext/memcached/php_memcached_server.c:793
#10 0x00007ffff75416d0 in zim_MemcachedServer_run (execute_data=<optimized out>, return_value=0x7fffffff9bd0) at /work/GIT/pecl-and-ext/memcached/php_memcached.c:3838
#11 0x000055555585691d in ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER () at /usr/src/debug/php-7.4.14-1.fc33.remi.x86_64/Zend/zend_vm_execute.h:1618
#12 execute_ex (ex=0x7ffff729be18) at /usr/src/debug/php-7.4.14-1.fc33.remi.x86_64/Zend/zend_vm_execute.h:53861
#13 0x0000555555858c7b in zend_execute (op_array=0x7ffff72892a0, return_value=0x0) at /usr/src/debug/php-7.4.14-1.fc33.remi.x86_64/Zend/zend_vm_execute.h:57957
#14 0x00005555557cf49c in zend_execute_scripts (type=type@entry=8, retval=0x7ffff72a5960, retval@entry=0x0, file_count=-148819936, file_count@entry=3)
    at /usr/src/debug/php-7.4.14-1.fc33.remi.x86_64/Zend/zend.c:1679
#15 0x000055555576c730 in php_execute_script (primary_file=<optimized out>) at /usr/src/debug/php-7.4.14-1.fc33.remi.x86_64/main/main.c:2621
#16 0x000055555585ad7a in do_cli (argc=11, argv=0x555555e21150) at /usr/src/debug/php-7.4.14-1.fc33.remi.x86_64/sapi/cli/php_cli.c:964
#17 0x000055555563c42e in main (argc=11, argv=0x555555e21150) at /usr/src/debug/php-7.4.14-1.fc33.remi.x86_64/sapi/cli/php_cli.c:1359

@remicollet
Copy link
Collaborator

Seems to do the work

@@ -296,6 +299,7 @@ protocol_binary_response_status s_flush_handler(const void *cookie, uint32_t whe
        }
 
        MEMC_MAKE_ZVAL_COOKIE(zcookie, cookie);
+       ZVAL_LONG(&zwhen, when);
 
        ZVAL_COPY(&params[0], &zcookie);
        ZVAL_COPY(&params[1], &zwhen);

m6w6 added a commit to m6w6/php-memcached that referenced this pull request Jan 19, 2021
@sodabrew
Copy link
Contributor

This looks really good! Please let me know when you're confident with this PR and I'll land it.

@sodabrew sodabrew added this to the 3.2.0 milestone Jan 19, 2021
@m6w6
Copy link
Contributor Author

m6w6 commented Jan 20, 2021

Thanks Remi!

This was quite a rabbit hole. The actual bug is/was in libmemcachedprotocol, see awesomized/libmemcached@2a67d99

@remicollet
Copy link
Collaborator

remicollet commented Jan 20, 2021

Was also searching a way to send the "END" message...

BTW shouln't we use in server.php
As discussed on IRC, for memory

$server->on (Memcached::ON_STAT,
    function ($client_id, $key, &$value) {
        echo "client_id=[$client_id]: Stat key=[$key]" . PHP_EOL;
        if ($key) {
            $value = "Stat reply for $key";
        } else {
            $value = ['foo' = > 'Stat Reply', 'bar' => 'Another'];
        }
    return Memcached::RESPONSE_SUCCESS;
});

@m6w6
Copy link
Contributor Author

m6w6 commented Jan 20, 2021

I'd prefer to always expect an array

@m6w6
Copy link
Contributor Author

m6w6 commented Feb 15, 2021

I think this should be fine now, or do you have anything to add @remicollet ?

@remicollet
Copy link
Collaborator

+1 LGTM

@remicollet
Copy link
Collaborator

Notice, this PR also eradicates the usage of inet_aton/inet_ntoa deprecated calls :)

@remicollet
Copy link
Collaborator

@m6w6 can you please rebase ?

@m6w6 m6w6 force-pushed the fix-memcachedserver branch from 8fb5ff9 to 10f4497 Compare July 23, 2021 12:55
@sodabrew sodabrew merged commit 9cd4a01 into php-memcached-dev:master Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants