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

Move mutex into world.status #690

Merged
merged 1 commit into from
Dec 27, 2023
Merged

Move mutex into world.status #690

merged 1 commit into from
Dec 27, 2023

Conversation

Bumber64
Copy link
Contributor

@Bumber64 Bumber64 commented Dec 24, 2023

Looks like the mutex unk_v5010_1166a8 is in world.status based on its use here (v50.11 win64 Steam, make_announcement function):

14005765b LEA       RSI,[RDI + 0x43c0]
140057662 MOV       qword ptr [RBP + -0x30],RSI
140057666 MOV       RCX,RSI
140057669 CALL      qword ptr [->MSVCP140.DLL::_Mtx_lock]

where RDI is world.status.

I renamed the mutex announcement_mutex, since world.status is announcement_handlerst. Not sure if the mutex has a purpose beyond that.

@ab9rf
Copy link
Member

ab9rf commented Dec 24, 2023

the mutex is actually the last element of announcement_handlerst and is called mtx in bay12 code

the status compound is actually called announcement and is of type announcement_handlerst. we could make announcement_handlerst its own top-level type without impacting any existing code (i think), but renaming status to announcement would require coordination which i don't think is worth the effort right now

as to the purpose, per a discussion i had with putnam, the mutex is required because the the worker threads used by the experimental multithreaded LOS code that she wrote can produce announcements (since dead body detection is handled by this code) and so a mutex is required to ensure that only a single worker is in the announcement vectors at a time

@Bumber64
Copy link
Contributor Author

Bumber64 commented Dec 25, 2023

Does this need a changelog at all, considering it's not something we need to touch (using our own thread locks?)

@ab9rf
Copy link
Member

ab9rf commented Dec 25, 2023

Does this need a changelog at all, considering it's not something we need to touch (using our own thread locks?)

in my opinion, no, but myk may feel otherwise

@myk002
Copy link
Member

myk002 commented Dec 27, 2023

Does this need a changelog at all, considering it's not something we need to touch (using our own thread locks?)

in my opinion, no, but myk may feel otherwise

No, I don't think this requires a changelog entry. Merging as-is

@myk002 myk002 merged commit 57401fa into DFHack:master Dec 27, 2023
17 checks passed
@Bumber64 Bumber64 deleted the patch-2 branch December 27, 2023 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants