You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by GitBox <gi...@apache.org> on 2022/12/02 08:46:42 UTC
[GitHub] [incubator-brpc] wwbmmm commented on a diff in pull request #1975: support SharedMutex.
wwbmmm commented on code in PR #1975:
URL: https://github.com/apache/incubator-brpc/pull/1975#discussion_r1037882245
##########
src/bthread/shared_mutex.h:
##########
@@ -0,0 +1,47 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#ifndef BTHREAD_SHARED_MUTEX_H
+#define BTHREAD_SHARED_MUTEX_H
+#include "mutex.h"
Review Comment:
前面加个空行
##########
src/bthread/shared_mutex.cpp:
##########
@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "shared_mutex.h"
+#include "butex.h"
+
+namespace bthread {
+SharedMutex::SharedMutex(): _reader_count(0), _reader_wait(0) {
Review Comment:
前面加个空行
##########
src/bthread/shared_mutex.cpp:
##########
@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "shared_mutex.h"
+#include "butex.h"
+
+namespace bthread {
+SharedMutex::SharedMutex(): _reader_count(0), _reader_wait(0) {
+ _writer_butex = butex_create_checked<uint32_t>();
+ *_writer_butex = 0;
+ _reader_butex = butex_create_checked<uint32_t>();
+ *_reader_butex = 0;
+}
Review Comment:
缺少析构函数实现，析构函数中应释放_reader_butex和_write_butex
##########
src/bthread/shared_mutex.cpp:
##########
@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "shared_mutex.h"
+#include "butex.h"
+
+namespace bthread {
+SharedMutex::SharedMutex(): _reader_count(0), _reader_wait(0) {
+ _writer_butex = butex_create_checked<uint32_t>();
+ *_writer_butex = 0;
+ _reader_butex = butex_create_checked<uint32_t>();
+ *_reader_butex = 0;
+}
+
+void SharedMutex::lock_shared() {
+ if (_reader_count.fetch_add(1) < 0) {
+ butex_wait(_reader_butex, 0, nullptr);
+ }
+}
+
+void SharedMutex::unlock_shared() {
+ int32_t r = _reader_count.fetch_add(-1);
+ if (r < 0) {
+ unlock_shared_slow(r);
+ }
+}
+
+void SharedMutex::unlock_shared_slow(int32_t r) {
+ if (r == 0 || r == -max_readers) {
+ throw std::system_error(std::error_code(static_cast<int>(std::errc::operation_not_permitted),
Review Comment:
可以改成brpc风格的CHECK
##########
src/bthread/shared_mutex.cpp:
##########
@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "shared_mutex.h"
+#include "butex.h"
+
+namespace bthread {
+SharedMutex::SharedMutex(): _reader_count(0), _reader_wait(0) {
+ _writer_butex = butex_create_checked<uint32_t>();
+ *_writer_butex = 0;
+ _reader_butex = butex_create_checked<uint32_t>();
+ *_reader_butex = 0;
+}
+
+void SharedMutex::lock_shared() {
+ if (_reader_count.fetch_add(1) < 0) {
+ butex_wait(_reader_butex, 0, nullptr);
+ }
+}
+
+void SharedMutex::unlock_shared() {
+ int32_t r = _reader_count.fetch_add(-1);
+ if (r < 0) {
+ unlock_shared_slow(r);
+ }
+}
+
+void SharedMutex::unlock_shared_slow(int32_t r) {
+ if (r == 0 || r == -max_readers) {
+ throw std::system_error(std::error_code(static_cast<int>(std::errc::operation_not_permitted),
+ std::system_category()),
+ "unlock of unlocked SharedMutex");
+ }
+ if (_reader_wait.fetch_add(-1) == 1) {
+ butex_wake(_writer_butex);
+ }
+}
+
+void SharedMutex::lock() {
+ _w.lock();
+ int32_t r = _reader_count.fetch_add(-max_readers);
+ if (r != 0 && _reader_wait.fetch_add(r) + r != 0) {
+ butex_wait(_writer_butex, 0, nullptr);
Review Comment:
wait结束，把write_butex的值置回0
##########
src/bthread/shared_mutex.cpp:
##########
@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "shared_mutex.h"
+#include "butex.h"
+
+namespace bthread {
+SharedMutex::SharedMutex(): _reader_count(0), _reader_wait(0) {
+ _writer_butex = butex_create_checked<uint32_t>();
+ *_writer_butex = 0;
+ _reader_butex = butex_create_checked<uint32_t>();
+ *_reader_butex = 0;
+}
+
+void SharedMutex::lock_shared() {
+ if (_reader_count.fetch_add(1) < 0) {
+ butex_wait(_reader_butex, 0, nullptr);
Review Comment:
wait结束后，把_reader_butex的值-1
##########
src/bthread/shared_mutex.cpp:
##########
@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "shared_mutex.h"
+#include "butex.h"
+
+namespace bthread {
+SharedMutex::SharedMutex(): _reader_count(0), _reader_wait(0) {
+ _writer_butex = butex_create_checked<uint32_t>();
+ *_writer_butex = 0;
+ _reader_butex = butex_create_checked<uint32_t>();
+ *_reader_butex = 0;
+}
+
+void SharedMutex::lock_shared() {
+ if (_reader_count.fetch_add(1) < 0) {
+ butex_wait(_reader_butex, 0, nullptr);
+ }
+}
+
+void SharedMutex::unlock_shared() {
+ int32_t r = _reader_count.fetch_add(-1);
+ if (r < 0) {
+ unlock_shared_slow(r);
+ }
+}
+
+void SharedMutex::unlock_shared_slow(int32_t r) {
+ if (r == 0 || r == -max_readers) {
+ throw std::system_error(std::error_code(static_cast<int>(std::errc::operation_not_permitted),
+ std::system_category()),
+ "unlock of unlocked SharedMutex");
+ }
+ if (_reader_wait.fetch_add(-1) == 1) {
+ butex_wake(_writer_butex);
+ }
+}
+
+void SharedMutex::lock() {
+ _w.lock();
+ int32_t r = _reader_count.fetch_add(-max_readers);
+ if (r != 0 && _reader_wait.fetch_add(r) + r != 0) {
+ butex_wait(_writer_butex, 0, nullptr);
+ }
+}
+
+void SharedMutex::unlock() {
+ int32_t r = _reader_count.fetch_add(max_readers);
+ for(int32_t i = 0; i < r; i++) {
+ butex_wake(_reader_butex);
Review Comment:
先把reader_butex的值加一
##########
src/bthread/shared_mutex.cpp:
##########
@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "shared_mutex.h"
+#include "butex.h"
+
+namespace bthread {
+SharedMutex::SharedMutex(): _reader_count(0), _reader_wait(0) {
+ _writer_butex = butex_create_checked<uint32_t>();
+ *_writer_butex = 0;
+ _reader_butex = butex_create_checked<uint32_t>();
+ *_reader_butex = 0;
+}
+
+void SharedMutex::lock_shared() {
+ if (_reader_count.fetch_add(1) < 0) {
+ butex_wait(_reader_butex, 0, nullptr);
+ }
+}
+
+void SharedMutex::unlock_shared() {
+ int32_t r = _reader_count.fetch_add(-1);
+ if (r < 0) {
+ unlock_shared_slow(r);
+ }
+}
+
+void SharedMutex::unlock_shared_slow(int32_t r) {
+ if (r == 0 || r == -max_readers) {
+ throw std::system_error(std::error_code(static_cast<int>(std::errc::operation_not_permitted),
+ std::system_category()),
+ "unlock of unlocked SharedMutex");
+ }
+ if (_reader_wait.fetch_add(-1) == 1) {
+ butex_wake(_writer_butex);
+ }
+}
+
+void SharedMutex::lock() {
+ _w.lock();
+ int32_t r = _reader_count.fetch_add(-max_readers);
+ if (r != 0 && _reader_wait.fetch_add(r) + r != 0) {
+ butex_wait(_writer_butex, 0, nullptr);
+ }
+}
+
+void SharedMutex::unlock() {
+ int32_t r = _reader_count.fetch_add(max_readers);
Review Comment:
这里要 + max_headers吧
##########
src/bthread/shared_mutex.cpp:
##########
@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "shared_mutex.h"
+#include "butex.h"
+
+namespace bthread {
+SharedMutex::SharedMutex(): _reader_count(0), _reader_wait(0) {
+ _writer_butex = butex_create_checked<uint32_t>();
+ *_writer_butex = 0;
+ _reader_butex = butex_create_checked<uint32_t>();
+ *_reader_butex = 0;
+}
+
+void SharedMutex::lock_shared() {
+ if (_reader_count.fetch_add(1) < 0) {
+ butex_wait(_reader_butex, 0, nullptr);
+ }
+}
+
+void SharedMutex::unlock_shared() {
+ int32_t r = _reader_count.fetch_add(-1);
+ if (r < 0) {
+ unlock_shared_slow(r);
+ }
+}
+
+void SharedMutex::unlock_shared_slow(int32_t r) {
+ if (r == 0 || r == -max_readers) {
+ throw std::system_error(std::error_code(static_cast<int>(std::errc::operation_not_permitted),
+ std::system_category()),
+ "unlock of unlocked SharedMutex");
+ }
+ if (_reader_wait.fetch_add(-1) == 1) {
+ butex_wake(_writer_butex);
Review Comment:
这里是不是得先更新_write_butex的值非0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org