Skip to content

Disable forkproxy by default#1353

Merged
sporksmith merged 2 commits intoshadow:devfrom
sporksmith:disable-forkproxy
May 12, 2021
Merged

Disable forkproxy by default#1353
sporksmith merged 2 commits intoshadow:devfrom
sporksmith:disable-forkproxy

Conversation

@sporksmith
Copy link
Copy Markdown
Contributor

@sporksmith sporksmith commented May 12, 2021 •
edited
Loading

  • Disable forkproxy / O(n)-waitpid-workaround by default
  • Fix a shutdown bug in thread_ptrace that this exposed

Closes #1261

@sporksmith sporksmith mentioned this pull request May 12, 2021
Closed
@codecov
Copy link
Copy Markdown

codecov bot commented May 12, 2021 •
edited
Loading

Codecov Report

Merging #1353 (8653886) into dev (09dc78b) will decrease coverage by 0.16%.
The diff coverage is 20.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #1353      +/-   ##
==========================================
- Coverage   56.59%   56.42%   -0.17%     
==========================================
  Files         141      141              
  Lines       20122    20123       +1     
  Branches     4997     4998       +1     
==========================================
- Hits        11388    11355      -33     
- Misses       5840     5879      +39     
+ Partials     2894     2889       -5
Flag Coverage Δ
tests 56.42% <20.00%> (-0.17%) ⬇️

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

Impacted Files Coverage Δ
src/main/host/thread_ptrace.c 51.16% <20.00%> (-1.26%) ⬇️
src/main/utility/fork_proxy.c 0.00% <0.00%> (-61.54%) ⬇️
src/main/core/logger/shadow_logger.c 80.76% <0.00%> (-0.97%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 09dc78b...8653886. Read the comment docs.

@robgjansen robgjansen added the Component: Main Composing the core Shadow executable label May 12, 2021
@sporksmith sporksmith force-pushed the disable-forkproxy branch 2 times, most recently from bd96e51 to 2f33b57 Compare May 12, 2021 19:39
@sporksmith sporksmith changed the title Disable forkproxy Disable forkproxy by default May 12, 2021
@sporksmith sporksmith requested a review from stevenengler May 12, 2021 19:42
robgjansen and others added 2 commits May 12, 2021 16:00
@robgjansen @sporksmith
Disable forkproxy workaround by default
bd9008d 
We want the default mode to be that Shadow calculates the number
of processors to use based on the number of hosts specified in the
configuration file. (See shadow#1251.) In this case, we do not want to
run the forkproxy workaround, because the fact that waitpid is O(n)
won't matter when each processor only runs a few pids.

Closes shadow#1261
@sporksmith
thread_ptrace: don't try to catch exit event if unattached
8653886 
When notified of process exit, we can't catch the ptrace exit event if
we're not still attached. Trying to do so will instead reap the final
exit status, which will cause the Process to fail to do so.
@sporksmith sporksmith force-pushed the disable-forkproxy branch from 2f33b57 to 8653886 Compare May 12, 2021 21:01
@sporksmith sporksmith merged commit a92bf6f into shadow:dev May 12, 2021
@sporksmith sporksmith deleted the disable-forkproxy branch May 12, 2021 22:55
@sporksmith sporksmith linked an issue May 12, 2021 that may be closed by this pull request
Remove or default-disable forkproxy waitpid workarounds #1261
Closed
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: Main Composing the core Shadow executable

Projects

None yet

Milestone

Fully support process-based simulatio...

Development

Successfully merging this pull request may close these issues.

Remove or default-disable forkproxy waitpid workarounds

3 participants

@sporksmith @stevenengler @robgjansen