Skip to content
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

Receiving trash from persistent connect when there was timeout before #113

Open
deadbabe opened this issue Feb 6, 2017 · 4 comments
Open
Labels
backlog bug Something isn't working php7 prio5

Comments

@deadbabe
Copy link

deadbabe commented Feb 6, 2017

When getting read timeout on persistent connect, socket is still alive and well and able to receive data.

This leads to getting trash from such connect on next request, or even receiving fully valid response, just not for the latest request, but for previous one.

This 06845559 kinda helps, but for complete safety it's might be better to reconnect on every uncertainty with connection - #112

Another workaround would be to mark timeout'd connects with a "dirty" flag and read out its buffer completely later, before next request or on php request shutdown.

bigbes added a commit that referenced this issue Feb 6, 2017
@bigbes
Copy link
Contributor

bigbes commented Feb 6, 2017

Hello @deadbabe . Thank you for your bug report. I can reproduce this bug with tarantool-3.ini suite (that'll test timeout case with persistent connection).

What had happened:

  1. opened persistent stream
  2. make persistent read (that'll block until timeout had happened)
  3. make php_stream_free(stream, PHP_STREAM_FREE_CLOSE_PERSISTENT)/php_stream_pclose(stream)
  4. open new stream with the same persistent id
  5. get still opened previous fd

I can't catch this bug under gdb:

  • php_sockop_close isn't called
  • if i'll put breakpoint to tntll_stream_close - everything works fine (it's a kind of magic)

This issues needs further investigation. As a workaround you can provide alternative persistent_id for new connection.

@bigbes bigbes self-assigned this Feb 6, 2017
@bigbes bigbes added bug Something isn't working php7 labels Feb 6, 2017
Totktonada added a commit that referenced this issue Mar 16, 2020
tarantool-3.ini configuration fails and it seems the reason is gh-133
('Receiving trash from persistent connect when there was timeout
before'). Disabled both tarantool-2.ini and tarantool-3.ini
configurations, where persistent connections are used, until the problem
will be resolved.

Related to #113.
Totktonada added a commit that referenced this issue Mar 18, 2020
tarantool-3.ini configuration fails and it seems the reason is gh-133
('Receiving trash from persistent connect when there was timeout
before'). Disabled both tarantool-2.ini and tarantool-3.ini
configurations, where persistent connections are used, until the problem
will be resolved.

Related to #113.
Totktonada added a commit that referenced this issue Mar 19, 2020
tarantool-3.ini configuration fails and it seems the reason is gh-133
('Receiving trash from persistent connect when there was timeout
before'). Disabled both tarantool-2.ini and tarantool-3.ini
configurations, where persistent connections are used, until the problem
will be resolved.

Related to #113.
Totktonada added a commit that referenced this issue Mar 20, 2020
tarantool-3.ini configuration fails and it seems the reason is gh-133
('Receiving trash from persistent connect when there was timeout
before'). Disabled both tarantool-2.ini and tarantool-3.ini
configurations, where persistent connections are used, until the problem
will be resolved.

Related to #113.
@Totktonada
Copy link
Member

Note (for myself): suffix/suffix_len are not stored in struct tarantool_connection, but used in __tarantool_connect() and tarantool_connection_free(). Mistake? Consider the following patch:

diff --git a/src/tarantool.c b/src/tarantool.c
index 91a9ac2..59d9e2d 100644
--- a/src/tarantool.c
+++ b/src/tarantool.c
@@ -1077,7 +1077,7 @@ PHP_METHOD(Tarantool, __construct) {
        long   port = 0;
 
        zend_bool is_persistent = false, plist_new_entry = true;
-       const char *suffix = NULL;
+       char *suffix = NULL;
        size_t suffix_len = 0;
        zend_string *plist_id = NULL;
 
@@ -1148,6 +1148,8 @@ PHP_METHOD(Tarantool, __construct) {
                /* CHECK obj->schema */
                obj->tps = tarantool_tp_new(obj->value, is_persistent);
                /* CHECK obj->tps */
+               obj->suffix = suffix;
+               obj->suffix_len = suffix_len;
        }
 
        if (is_persistent && plist_new_entry) {

This, however, does not fixes the problem on tarantool-3.ini testing configuration.

@Totktonada
Copy link
Member

NB: When the obj (struct tarantool_connection) can be non-NULL? Copying? Is it handled right?

Totktonada added a commit that referenced this issue Mar 23, 2020
tarantool-3.ini configuration fails and it seems the reason is gh-113
('Receiving trash from persistent connect when there was timeout
before'). Disabled both tarantool-2.ini and tarantool-3.ini
configurations, where persistent connections are used, until the problem
will be resolved.

Related to #113.
@Totktonada Totktonada added this to the PHP 7.* + bugfixes milestone Mar 23, 2020
Totktonada added a commit that referenced this issue Mar 28, 2020
tarantool-3.ini configuration fails and it seems the reason is gh-113
('Receiving trash from persistent connect when there was timeout
before'). Disabled both tarantool-2.ini and tarantool-3.ini
configurations, where persistent connections are used, until the problem
will be resolved.

Related to #113.
@Totktonada
Copy link
Member

See also tarantool-php/client@ee6bc94

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog bug Something isn't working php7 prio5
Projects
None yet
Development

No branches or pull requests

4 participants