Opened 4 months ago

Closed 4 months ago

#29541 closed defect (fixed)

Re-enable util/mmap_anon_no_fork

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.4.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: prop289, prop289-assigned-sponsor-v, 041-proposed-on-roadmap, network-team-roadmap-2019-Q1Q2, asn-merge
Cc: teor Actual Points:
Parent ID: #26288 Points:
Reviewer: mikeperry Sponsor:

Description

The test above was disabled with #29534 in 0.4.0. We should re-enable it and make it pass everywhere. I think I know what the problem is: I think it's a matter of some flags existing in the headers but not in the kernel.

Child Tickets

Change History (11)

comment:1 Changed 4 months ago by nickm

Cc: teor added
Status: assignedneeds_review

I have a patch at #29541 to fix the test and re-enable it. It works for me on Fedora 29 and on OSX 10.14.3. I've made a PR at https://github.com/torproject/tor/pull/723 -- let's see if the CI passes.

comment:2 Changed 4 months ago by asn

Reviewer: mikeperry

comment:3 Changed 4 months ago by teor

I tested commit:

commit 5614960e94 (HEAD, tor-github/pr/723/merge)
Merge: 69238ca2da 065e7da8e6

util/mmap_anon_no_fork passes on my machine.

But #29585 and #29586 occurred once in about 10 test runs. They happened in the same test run. Could this change have affected the reliability of these other tests?

comment:4 Changed 4 months ago by nickm

I really don't see how this test bug could have affected those tests... but the world has been known to do things that I do not expect. If they happen one time in ten, maybe checking to see whether this fix fixes them too might be reasonable.

comment:5 Changed 4 months ago by dgoulet

Keywords: prop289 prop289-assigned-sponsor-v removed
Parent ID: #26288

I have no clue how this relates to prop289 (#26288). Feel free to re-parent but explains why before?

comment:6 Changed 4 months ago by nickm

This is prop289 because it's a bug in the no-fork mmap tests; I added the no-fork mmap to make the fast PRNG more fork-safe.

comment:7 Changed 4 months ago by teor

Keywords: prop289 prop289-assigned-sponsor-v added
Parent ID: #26288

Restoring prop289

comment:8 in reply to:  4 Changed 4 months ago by teor

Replying to nickm:

I really don't see how this test bug could have affected those tests... but the world has been known to do things that I do not expect. If they happen one time in ten, maybe checking to see whether this fix fixes them too might be reasonable.

To be clear, #29585 and #29586 happened once. And then they haven't happened again. I think they may have happened due to a clock change during the test run. If they happen again, we can reopen those tickets.

comment:9 Changed 4 months ago by mikeperry

Status: needs_reviewmerge_ready

This is a little mind-bendy with the magic behavior constants and bytes being sent back and forth, but it's well commented and looks ok.

comment:10 Changed 4 months ago by nickm

Keywords: asn-merge added

comment:11 Changed 4 months ago by asn

Resolution: fixed
Status: merge_readyclosed

Merged. There was no changes file but that's because the bug was not released yet.

Note: See TracTickets for help on using tickets.