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

Data race in TestConnection_NewWatcher_concurrent #284

Closed
better0fdead opened this issue Apr 19, 2023 · 0 comments · Fixed by #289
Closed

Data race in TestConnection_NewWatcher_concurrent #284

better0fdead opened this issue Apr 19, 2023 · 0 comments · Fixed by #289
Assignees
Labels
bug Something isn't working teamE

Comments

@better0fdead
Copy link

Have faced a data race in github ci run.

==================
WARNING: DATA RACE
Write at 0x00c00005e750 by goroutine 91:
  github.com/tarantool/go-tarantool_test.TestConnection_NewWatcher_concurrent.func1()
      /Users/runner/work/go-tarantool/go-tarantool/tarantool/go-tarantool/tarantool_test.go:3854 +0x377

Previous write at 0x00c00005e750 by goroutine 203:
  github.com/tarantool/go-tarantool_test.TestConnection_NewWatcher_concurrent.func1()
      /Users/runner/work/go-tarantool/go-tarantool/tarantool/go-tarantool/tarantool_test.go:3854 +0x377

Goroutine 91 (running) created at:
  github.com/tarantool/go-tarantool_test.TestConnection_NewWatcher_concurrent()
      /Users/runner/work/go-tarantool/go-tarantool/tarantool/go-tarantool/tarantool_test.go:3840 +0x435
  testing.tRunner()
      /Users/runner/hostedtoolcache/go/1.13.15/x64/src/testing/testing.go:909 +0x199

Goroutine 203 (running) created at:
  github.com/tarantool/go-tarantool_test.TestConnection_NewWatcher_concurrent()
      /Users/runner/work/go-tarantool/go-tarantool/tarantool/go-tarantool/tarantool_test.go:3840 +0x435
  testing.tRunner()
      /Users/runner/hostedtoolcache/go/1.13.15/x64/src/testing/testing.go:909 +0x199
==================
--- FAIL: TestConnection_NewWatcher_concurrent (1.49s)
    tarantool_test.go:3863: An error found: Unable to get an event 49
    testing.go:853: race detected during execution of test
@better0fdead better0fdead added bug Something isn't working teamE labels Apr 19, 2023
oleg-jukovec added a commit that referenced this issue May 10, 2023
Goroutines could set a value to `ret` shared variable without
protection. The shared variable has been replaced with a channel.

Part of #284
oleg-jukovec added a commit that referenced this issue May 10, 2023
Watchers may behave incorrectly if a request timeout is too small. A
re-IPROTO_WATCH request may be not send to a server. It could lead to
loss of events stream.

It also could lead to a lost IPROTO_UNREGISTER request, but a user
won't see the problem.

Closes #284
oleg-jukovec added a commit that referenced this issue May 10, 2023
This helps to keep a send order for async requests if they send in the
order.

Part of #284
oleg-jukovec added a commit that referenced this issue May 10, 2023
Watchers may behave incorrectly if a request timeout is too small. A
re-IPROTO_WATCH request may be not send to a server. It could lead to
loss of events stream.

It also could lead to a lost IPROTO_UNREGISTER request, but a user
won't see the problem.

Closes #284
oleg-jukovec added a commit that referenced this issue May 10, 2023
The change helps to make the order of execution and sending of async
requests the same.

As example:

    conn.Do(AsyncRequest1).Get()
    conn.Do(AsyncRequest2).Get()

It is now guaranteed that AsyncRequest2 will be sent to the network
after AsyncRequest1. Before the patch the order was undefined.

Part of #284
oleg-jukovec added a commit that referenced this issue May 10, 2023
Watchers may behave incorrectly if a request timeout is too small. A
re-IPROTO_WATCH request may be not send to a server. It could lead to
loss of events stream.

It also could lead to a lost IPROTO_UNREGISTER request, but a user
won't see the problem.

Closes #284
@oleg-jukovec oleg-jukovec self-assigned this May 11, 2023
oleg-jukovec added a commit that referenced this issue May 17, 2023
Goroutines could set a value to `ret` shared variable without
protection. The shared variable has been replaced with a channel.

Part of #284
oleg-jukovec added a commit that referenced this issue May 17, 2023
The change helps to make the order of execution and sending of async
requests the same.

As example:

    conn.Do(AsyncRequest1).Get()
    conn.Do(AsyncRequest2).Get()

It is now guaranteed that AsyncRequest2 will be sent to the network
after AsyncRequest1. Before the patch the order was undefined.

Part of #284
oleg-jukovec added a commit that referenced this issue May 17, 2023
Watchers may behave incorrectly if a request timeout is too small. A
re-IPROTO_WATCH request may be not send to a server. It could lead to
loss of events stream.

It also could lead to a lost IPROTO_UNREGISTER request, but a user
won't see the problem.

Closes #284
oleg-jukovec added a commit that referenced this issue May 18, 2023
Overview

    The release adds pagination support and wrappers for the
    crud module.

Breaking changes

    There are no breaking changes in the release.

New features

    Support pagination (#246).

    A Makefile target to test with race detector (#218).

    Support CRUD API (#108).

    An ability to replace a base network connection to a Tarantool
    instance (#265).

    Missed iterator constant (#285).

Bugfixes

    Several non-critical data race issues (#218).

    Build on Apple M1 with OpenSSL (#260).

    ConnectionPool does not properly handle disconnection with
    Opts.Reconnect set (#272).

    Watcher events loss with a small per-request timeout (#284).

    Connect() panics on concurrent schema update (#278).

    Wrong Ttr setup by Queue.Cfg() (#278).

    Flaky queue/Example_connectionPool (#278).

    Flaky queue/Example_simpleQueueCustomMsgPack (#277).

Other

    queue module version bumped to 1.3.0 (#278).
oleg-jukovec added a commit that referenced this issue May 18, 2023
Overview

    The release adds pagination support and wrappers for the
    crud module.

Breaking changes

    There are no breaking changes in the release.

New features

    Support pagination (#246).

    A Makefile target to test with race detector (#218).

    Support CRUD API (#108).

    An ability to replace a base network connection to a Tarantool
    instance (#265).

    Missed iterator constant (#285).

Bugfixes

    Several non-critical data race issues (#218).

    Build on Apple M1 with OpenSSL (#260).

    ConnectionPool does not properly handle disconnection with
    Opts.Reconnect set (#272).

    Watcher events loss with a small per-request timeout (#284).

    Connect() panics on concurrent schema update (#278).

    Wrong Ttr setup by Queue.Cfg() (#278).

    Flaky queue/Example_connectionPool (#278).

    Flaky queue/Example_simpleQueueCustomMsgPack (#277).

Other

    queue module version bumped to 1.3.0 (#278).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working teamE
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants