Skip to content

Make ThreadRef the owner of the C Thread and rename to Thread#2705

Merged
sporksmith merged 11 commits intoshadow:mainfrom
sporksmith:thread-rust
Jan 31, 2023
Merged

Make ThreadRef the owner of the C Thread and rename to Thread#2705
sporksmith merged 11 commits intoshadow:mainfrom
sporksmith:thread-rust

Conversation

@sporksmith
Copy link
Copy Markdown
Contributor

@sporksmith sporksmith commented Jan 27, 2023

No description provided.

@sporksmith sporksmith self-assigned this Jan 27, 2023
@github-actions github-actions bot added the Component: Main Composing the core Shadow executable label Jan 27, 2023
@sporksmith sporksmith force-pushed the thread-rust branch 2 times, most recently from e0431d5 to 00923f4 Compare January 27, 2023 21:45
@sporksmith
Copy link
Copy Markdown
Contributor Author

sporksmith commented Jan 27, 2023

Sorry, jumped the gun again on requesting review. Taking a look at the failing test...

@sporksmith sporksmith removed the request for review from stevenengler January 27, 2023 21:59
@github-actions github-actions bot added Component: Build Build/install tools and dependencies Component: Testing Unit and integration tests and frameworks labels Jan 27, 2023
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 27, 2023 •
edited
Loading

Codecov Report

Base: 67.58% // Head: 68.07% // Increases project coverage by +0.49% 🎉

Coverage data is based on head (66a66a5) compared to base (9a66b2f).
Patch coverage: 90.00% of modified lines in pull request are covered.

❗ Current head 66a66a5 differs from pull request most recent head 8cc9964. Consider uploading reports for the commit 8cc9964 to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2705      +/-   ##
==========================================
+ Coverage   67.58%   68.07%   +0.49%     
==========================================
  Files         203      202       -1     
  Lines       30472    30410      -62     
  Branches     5960     5949      -11     
==========================================
+ Hits        20595    20703     +108     
+ Misses       5201     5004     -197     
- Partials     4676     4703      +27
Flag Coverage Δ
tests 68.07% <90.00%> (+0.49%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/main/host/context.rs 62.50% <83.33%> (+1.20%) ⬆️
src/main/host/process.rs 79.91% <88.23%> (+1.25%) ⬆️
src/main/core/worker.rs 82.03% <100.00%> (+4.65%) ⬆️
src/main/host/memory_manager/memory_mapper.rs 71.60% <100.00%> (+0.19%) ⬆️
src/main/host/memory_manager/mod.rs 67.74% <100.00%> (ø)
src/main/host/syscall_condition.rs 84.21% <100.00%> (-0.54%) ⬇️
src/main/host/thread.rs 80.98% <100.00%> (-0.14%) ⬇️
src/main/host/network_interface.rs 76.92% <0.00%> (-14.46%) ⬇️
src/main/network/router/mod.rs 75.00% <0.00%> (-11.00%) ⬇️
src/main/utility/shm_cleanup.rs 59.09% <0.00%> (-2.28%) ⬇️
... and 28 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

stevenengler
stevenengler approved these changes Jan 31, 2023
View reviewed changes
@sporksmith
Process::threads: remove stale comment
93a2fc6
@sporksmith
SysCallCondition: store a tid instead of a Thread*
11bb571 
This is a step towards making Thread itself not ref-counted, for its
transition to Rust.
@sporksmith
SysCallHandler: don't increment Thread refcount
4080fb8 
We already have a comment in `struct _SysCallHandler` stating that the
handler must not outlive the Thread.
@sporksmith
ThreadRef: don't manipulate Thread refcount
682d165 
This makes ThreadRef a "borrowing" accessor to Thread, since it no
longer necessarily "owns" the pointer.
@sporksmith
Remove thread_ref
3fa5f3b
@sporksmith
Rename thread_unref -> thread_free
b6e4be3
@sporksmith
Remove Thread.referenceCount
a0b775e
@sporksmith
Stop constructing ephemeral ThreadRef's
1f7c827 
We want to make ThreadRef own the cshadow::Thread, so we can no longer
construct ephemeral ones on the fly.
@sporksmith
Make ThreadRef the owner of its cshadow::Thread
47bfe72
@sporksmith
Rename ThreadRef -> Thread
e02d9ee
@sporksmith
Add a smaller test for cross-process signals
8cc9964 
An earlier version of this PR had a bug in cross-process signals that
was only caught by one of the extra tests (basic-file-transfer).
This is a more focused test for the corresponding functionality.
@sporksmith sporksmith enabled auto-merge January 31, 2023 15:33
@sporksmith sporksmith merged commit 88e32b7 into shadow:main Jan 31, 2023
@sporksmith sporksmith deleted the thread-rust branch January 31, 2023 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stevenengler stevenengler stevenengler approved these changes

Assignees

@sporksmith sporksmith

Labels

Component: Build Build/install tools and dependencies Component: Main Composing the core Shadow executable Component: Testing Unit and integration tests and frameworks

Projects

None yet

Milestone

Convert existing modules to Rust

Development

Successfully merging this pull request may close these issues.

2 participants

@sporksmith @stevenengler