Skip to content

Conversation

@kevinAlbs
Copy link
Collaborator

@kevinAlbs kevinAlbs commented May 25, 2020

This is not urgent. This, and some other test fixes from #607 are prerequisites for adding a thread-sanitizer task to evergreen to automatically check for thread related bugs.

mock_server_hangs_up and mock_server_resets were closing a request stream directly from the main thread. But a request stream should only be accessed by the worker thread. This now adds a special reply to the worker thread's queue to signal a hangup or reset.

kevinAlbs added 2 commits May 25, 2020 14:14
mock_server_hangs_up and mock_server_resets were closing a
request stream directly from the main thread. But a request
stream should only be accessed by the worker thread. This
now adds a special reply to the worker thread's queue
to signal a hangup or reset.
mongoc_stream_close (request->client);
reply = bson_malloc0 (sizeof (reply_t));
reply->type = HANGUP;
q_put (request->replies, reply);
Copy link
Collaborator Author

@kevinAlbs kevinAlbs May 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior of mock_server_hangs_up was to close the server side stream from the main thread. TSan flags a data race because in the process of closing, it overwrites the stream's file descriptor with an invalid one, causing a write/read race.

The mock server maintains a worker thread for each server-side connection. And each worker thread maintains a thread-safe queue for sending replies back. So instead of closing the stream directly, this pushes a special reply onto the queue to signal the worker thread to close the stream.

@kevinAlbs kevinAlbs requested a review from rcsanchez97 May 26, 2020 12:42
Copy link
Contributor

@rcsanchez97 rcsanchez97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kevinAlbs kevinAlbs merged commit a562d82 into mongodb:master May 29, 2020
kevinAlbs added a commit that referenced this pull request May 29, 2020
mock_server_hangs_up and mock_server_resets were closing a
request stream directly from the main thread. But a request
stream should only be accessed by the worker thread. This
now adds a special reply to the worker thread's queue
to signal a hangup or reset.
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.

2 participants