Skip to content

support SharedMutex.#1975

Closed
lycplus wants to merge 1 commit into
apache:masterfrom
lycplus:shared_mutex
Closed

support SharedMutex.#1975
lycplus wants to merge 1 commit into
apache:masterfrom
lycplus:shared_mutex

Conversation

@lycplus
Copy link
Copy Markdown

@lycplus lycplus commented Oct 30, 2022

support SharedMutex that is compatible with c++17 std::shared_mutex. This patch uses existing class Mutex, and is migrated from golang, see https://github.com/golang/go/blob/master/src/sync/rwmutex.go

@wwbmmm
Copy link
Copy Markdown
Contributor

wwbmmm commented Oct 31, 2022

related: #1031

@wwbmmm
Copy link
Copy Markdown
Contributor

wwbmmm commented Oct 31, 2022

CI is failed

@lycplus
Copy link
Copy Markdown
Author

lycplus commented Dec 1, 2022

@wwbmmm I have fixed the build, the CI failure seems unrelated to this patch, please help check it.

@wwbmmm
Copy link
Copy Markdown
Contributor

wwbmmm commented Dec 1, 2022

@guodongxiaren Please help to see this CI failure, the error is so strange.

@guodongxiaren
Copy link
Copy Markdown
Member

guodongxiaren commented Dec 1, 2022

I have fixed this issue in #2030 @wwbmmm please review and merge in master.

@wwbmmm
Copy link
Copy Markdown
Contributor

wwbmmm commented Dec 2, 2022

@wwbmmm I have fixed the build, the CI failure seems unrelated to this patch, please help check it.

@liyichao the master branch has fixed the CI issue, please merge master and retry

@lycplus
Copy link
Copy Markdown
Author

lycplus commented Dec 2, 2022

@wwbmmm I have merged master, the workflows need approval

Comment thread src/bthread/shared_mutex.h
Comment thread src/bthread/shared_mutex.cpp
Comment thread src/bthread/shared_mutex.cpp
Comment thread src/bthread/shared_mutex.cpp Outdated
Comment thread src/bthread/shared_mutex.cpp
Comment thread src/bthread/shared_mutex.cpp Outdated
Comment thread src/bthread/shared_mutex.cpp Outdated
Comment thread src/bthread/shared_mutex.cpp
Comment thread src/bthread/shared_mutex.cpp Outdated

static constexpr int32_t max_readers = 1 << 30;
Mutex _w;
uint32_t* _writer_butex;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

改成butil::atomic<uint32_t>*,保证操作的内存序

static constexpr int32_t max_readers = 1 << 30;
Mutex _w;
uint32_t* _writer_butex;
uint32_t* _reader_butex;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

同上

@wwbmmm
Copy link
Copy Markdown
Contributor

wwbmmm commented Dec 6, 2022

建议补充单测

Copy link
Copy Markdown
Member

@chenzhangyi chenzhangyi left a comment

Choose a reason for hiding this comment

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

需要补充单测和注释,特别是牵涉到atomic的部分。 要注释里面说清楚为什么这种实现在所有组合下都是线程安全的。

@chenBright
Copy link
Copy Markdown
Contributor

Closed this as completed in #2752.

@chenBright chenBright closed this Sep 26, 2024
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.

5 participants