Make the thread_signal_deliverable comment match its acquire loads

Đang mở Phù hợp với người mới
  • Có gì không ổn?
#531 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
1/5
Thời gian dự kiến
Dưới một giờ
Mức phù hợp với người mới
80/100
Loại issue
Tài liệu
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
c
Lĩnh vực
documentation

Hướng nghiên cứu

Đọc chú thích phía trên thread_signal_deliverable trong src/runtime/thread.h, sau đó đọc phần quét trong src/runtime/thread.c dùng THREAD_FOR_EACH_ACTIVE_RELAXED và thread_blocked_load. Kiểm tra commit 7bfe357 để biết vì sao các acquire load là có chủ đích, và giữ lại phần giải thích vì sao các lần đọc cũ là vô hại. Hoàn thành nghĩa là header không còn khẳng định các lần đọc relaxed nữa, và nếu bạn thực hiện việc đổi tên tùy chọn, mọi kết quả grep cho macro đều được cập nhật và cây vẫn build được.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

Problem

The comment above thread_signal_deliverable in src/runtime/thread.h says it "uses relaxed reads on per-thread blocked fields". The code does not: the scan in src/runtime/thread.c walks THREAD_FOR_EACH_ACTIVE_RELAXED, which loads active with memory_order_acquire, and reads each mask through thread_blocked_load, which also uses memory_order_acquire. A reader who trusts the header comment would assume weaker ordering than the code gives, and the macro name ..._RELAXED points the same wrong way.

Proposal

Drop the ordering claim from the header comment and describe it as the lock-free scan it is ("Lock-free scan of the per-thread blocked fields; ..."), keeping the existing text on why stale reads are benign. Optionally rename THREAD_FOR_EACH_ACTIVE_RELAXED to THREAD_FOR_EACH_ACTIVE_LOCKFREE, since "relaxed" there means "without thread_lock", not memory_order_relaxed. Comment-only, or a mechanical rename; no behavior change.

Alternatives

Change the loads to memory_order_relaxed to match the comment. Not proposed: the acquire loads came in with 7bfe357 ("Move shared state to C11 atomics"), and nothing here shows they are unneeded.

Origin

Found during review of #505, where cubic flagged the same wording on the new thread_signal_blocked_any comment and it was fixed there.

Ngôn ngữ chính
C
Star
282
Fork
32
Merge trung bình
1 ngày 12 giờ
Pull request đã merge (30 ngày)
17

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của sysprog21/elfuse

Tất cả issue của sysprog21/elfuse

Issue tương tự

Thêm issue về C

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.