February 2020 - tor-commits - lists.torproject.org

[chutney/master] Merge remote-tracking branch 'tor-github/pr/55'
by teor＠torproject.org 19 Feb '20

19 Feb '20

commit 93091c1a93ea6695fdbc3b45a1e923016b9f7636 Merge: 0f39f6c cf4613f Author: teor <teor(a)torproject.org> Date: Wed Feb 19 10:12:11 2020 +1000 Merge remote-tracking branch 'tor-github/pr/55' .travis.yml | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)

1 0

[tor/master] Makefile: Add v3 onion services to the mixed network
by teor＠torproject.org 19 Feb '20

19 Feb '20

commit 85eb170923bdc7b42db971fd35e59281333931a2 Author: teor <teor(a)torproject.org> Date: Mon Feb 17 14:49:13 2020 +1000 Makefile: Add v3 onion services to the mixed network This network is used in make test-network-all and test-network-ipv4. Part of 33334. --- src/test/include.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/include.am b/src/test/include.am index fdd1195fd..9b5376d87 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -72,12 +72,12 @@ TEST_CHUTNEY_FLAVOR_QUICK_IPV6 = client-ipv6-only-md TEST_CHUTNEY_FLAVORS = basic-min bridges-min hs-v2-min hs-v3-min \ single-onion-v23 -# only run if we can ping6 ::1 (localhost) +# only run if we can ping ::1 (localhost) TEST_CHUTNEY_FLAVORS_IPV6 = bridges+ipv6-min ipv6-exit-min hs-v23-ipv6-md \ single-onion-v23-ipv6-md # only run if we can find a stable (or simply another) version of tor -TEST_CHUTNEY_FLAVORS_MIXED = mixed+hs-v2 +TEST_CHUTNEY_FLAVORS_MIXED = mixed+hs-v23 ### This is a lovely feature, but it requires automake >= 1.12, and Tor ### doesn't require that yet.

1 0

[tor/master] Makefile: Refactor and tweak test-network
by teor＠torproject.org 19 Feb '20

19 Feb '20

commit 2151822bd054a0c728194a6f47678f25d3ada8a8 Author: teor <teor(a)torproject.org> Date: Mon Feb 17 14:26:41 2020 +1000 Makefile: Refactor and tweak test-network The refactor is simpler, and supports IPv6 mixed networks. The refactor also slightly changes the order of the IPv6 and mixed checks. But it's closer to the original order before 33280. And if all the network lists that need a network type are empty, we don't check if we can run that network type. (And if a network list is empty, we don't say we're going to run an empty list.) Part of 33334. --- Makefile.am | 137 +++++++++++++++++++++++++++++++++--------------------------- 1 file changed, 76 insertions(+), 61 deletions(-) diff --git a/Makefile.am b/Makefile.am index 45dab1e6b..7e57570a0 100644 --- a/Makefile.am +++ b/Makefile.am @@ -303,9 +303,8 @@ need-chutney-path: test-network: @$(MAKE) test-network-mkdir @$(MAKE) test-network-clean - @$(MAKE) test-network-ipv4-impl \ - ipv4_flavors="$(TEST_CHUTNEY_FLAVOR_QUICK)" - @$(MAKE) test-network-ipv6-impl \ + @$(MAKE) test-network-run \ + ipv4_flavors="$(TEST_CHUTNEY_FLAVOR_QUICK)" \ ipv6_flavors="$(TEST_CHUTNEY_FLAVOR_QUICK_IPV6)" @$(MAKE) test-network-results @@ -315,11 +314,9 @@ test-network: test-network-all: @$(MAKE) test-network-mkdir @$(MAKE) test-network-clean - @$(MAKE) test-network-ipv4-impl \ - ipv4_flavors="$(TEST_CHUTNEY_FLAVORS)" - @$(MAKE) test-network-mixed-impl \ - mixed_flavors="$(TEST_CHUTNEY_FLAVORS_MIXED)" - @$(MAKE) test-network-ipv6-impl \ + @$(MAKE) test-network-run \ + ipv4_flavors="$(TEST_CHUTNEY_FLAVORS)" \ + mixed_flavors="$(TEST_CHUTNEY_FLAVORS_MIXED)" \ ipv6_flavors="$(TEST_CHUTNEY_FLAVORS_IPV6)" @$(MAKE) test-network-results @@ -329,8 +326,8 @@ test-network-all: test-network-ipv4: @$(MAKE) test-network-mkdir @$(MAKE) test-network-clean - @$(MAKE) test-network-ipv4-impl ipv4_flavors="$(TEST_CHUTNEY_FLAVORS)" - @$(MAKE) test-network-mixed-impl \ + @$(MAKE) test-network-run \ + ipv4_flavors="$(TEST_CHUTNEY_FLAVORS)" \ mixed_flavors="$(TEST_CHUTNEY_FLAVORS_MIXED)" @$(MAKE) test-network-results @@ -340,7 +337,7 @@ test-network-ipv4: test-network-ipv6: @$(MAKE) test-network-mkdir @$(MAKE) test-network-clean - @$(MAKE) test-network-ipv6-impl \ + @$(MAKE) test-network-run \ ipv6_flavors="$(TEST_CHUTNEY_FLAVORS_IPV6)" @$(MAKE) test-network-results @@ -359,69 +356,87 @@ test-network-clean: rm -f "$(TEST_NETWORK_ALL_LOG_DIR)"/*.log \ "$(TEST_NETWORK_ALL_LOG_DIR)"/*.trs -# Run the IPv4-only test flavors -# - $(ipv4_flavors) contains the tests to run -.PHONY: test-network-ipv4-impl -.SILENT: test-network-ipv4-impl -test-network-ipv4-impl: - @$(MAKE) test-network-run flavors="$(ipv4_flavors)" skip_flavors="" - -# Run the mixed tests in $(mixed_flavors), if a tor-stable binary is available -# - only run mixed tests if we have a tor-stable binary -# - $(mixed_flavors) contains the tests to run (or skip) -.PHONY: test-network-mixed-impl -.SILENT: test-network-mixed-impl -test-network-mixed-impl: - @flavors=""; \ - if command -v tor-stable >/dev/null 2>&1; then \ - echo "tor-stable found, running mixed flavors:" \ - "$(mixed_flavors)."; \ - flavors="$$flavors $(mixed_flavors)"; \ - else \ - echo "tor-stable not found, skipping mixed flavors:" \ - "$(mixed_flavors)."; \ - skip_flavors="$$skip_flavors $(mixed_flavors)"; \ - fi; \ - $(MAKE) test-network-run flavors="$$flavors" \ - skip_flavors="$$skip_flavors" - +# Run tests using automake's test-driver +# When checking if a set of test can be run, log the type of test, and the +# list of tests that will be run (or skipped). +# +# Run the IPv4 tests in $(ipv4_flavors), unconditionally +# - tor relays and directory authorities require IPv4. # Run the IPv6 tests in $(ipv6_flavors), if IPv6 is available # - only run IPv6 tests if we can ping6 or ping -6 ::1 (localhost) # we try the syntax for BSD ping6, Linux ping6, and Linux ping -6, # because they're incompatible # - some IPv6 tests may fail without an IPv6 DNS server # (see #16971 and #17011) -# - $(ipv6_flavors) contains the tests to run (or skip) -.PHONY: test-network-ipv6-impl -.SILENT: test-network-ipv6-impl -test-network-ipv6-impl: - @flavors=""; \ - if ping6 -q -c 1 -o ::1 >/dev/null 2>&1 || \ - ping6 -q -c 1 -W 1 ::1 >/dev/null 2>&1 || \ - ping -6 -c 1 -W 1 ::1 >/dev/null 2>&1; then \ - echo "ping6 ::1 or ping ::1 succeeded, running IPv6" \ - "flavors: $(ipv6_flavors)."; \ - flavors="$$flavors $(ipv6_flavors)"; \ - else \ - echo "ping6 ::1 and ping ::1 failed, skipping IPv6 flavors:" \ - "$(ipv6_flavors)."; \ - skip_flavors="$$skip_flavors $(ipv6_flavors)"; \ - fi; \ - $(MAKE) test-network-run flavors="$$flavors" \ - skip_flavors="$$skip_flavors" - -# Run tests using automake's test-driver -# - $(flavors) contains the tests to run -# - $(skip_flavors) contains the tests to skip +# Run the mixed tests in $(mixed_flavors), if a tor-stable binary is available +# - only run mixed tests if we have a tor-stable binary +# Run the IPv6 mixed tests in $(ipv6_mixed_flavors), if IPv6 and mixed are run +# - see above for details about IPv6 and mixed .PHONY: test-network-run .SILENT: test-network-run # We need the word splitting in the "for" lines, so we can't quote # $(skip_flavors) or $(flavors) test-network-run: need-chutney-path test-driver $(TESTING_TOR_BINARY) src/tools/tor-gencert - @for f in $(skip_flavors); do \ + @flavors=""; \ + skip_flavors=""; \ + if test -n "$(ipv4_flavors)"; then \ + echo "Running IPv4 flavors: $(ipv4_flavors)."; \ + flavors="$$flavors $(ipv4_flavors)"; \ + fi; \ + test_network_ipv6=false; \ + if test -n "$(ipv6_flavors)" || \ + test -n "$(ipv6_mixed_flavors)"; then \ + if ping6 -q -c 1 -o ::1 >/dev/null 2>&1 || \ + ping6 -q -c 1 -W 1 ::1 >/dev/null 2>&1 || \ + ping -6 -c 1 -W 1 ::1 >/dev/null 2>&1; then \ + test_network_ipv6=true; \ + fi; \ + fi; \ + if test -n "$(ipv6_flavors)"; then \ + if test "$$test_network_ipv6" = "true"; then \ + echo "ping6 ::1 or ping ::1 succeeded, running IPv6" \ + "flavors: $(ipv6_flavors)."; \ + flavors="$$flavors $(ipv6_flavors)"; \ + else \ + echo "ping6 ::1 and ping ::1 failed, skipping IPv6" \ + "flavors: $(ipv6_flavors)."; \ + skip_flavors="$$skip_flavors $(ipv6_flavors)"; \ + fi; \ + fi; \ + test_network_mixed=false; \ + if test -n "$(mixed_flavors)" || \ + test -n "$(ipv6_mixed_flavors)"; then \ + if command -v tor-stable >/dev/null 2>&1; then \ + test_network_mixed=true; \ + fi; \ + fi; \ + if test -n "$(mixed_flavors)"; then \ + if test "$$test_network_mixed" = "true"; then \ + echo "tor-stable found, running mixed flavors:" \ + "$(mixed_flavors)."; \ + flavors="$$flavors $(mixed_flavors)"; \ + else \ + echo "tor-stable not found, skipping mixed flavors:" \ + "$(mixed_flavors)."; \ + skip_flavors="$$skip_flavors $(mixed_flavors)"; \ + fi; \ + fi; \ + if test -n "$(ipv6_mixed_flavors)"; then \ + if test "$$test_network_ipv6" = "true" && \ + test "$$test_network_mixed" = "true"; then \ + echo "Running IPv6 mixed flavors:" \ + "$(ipv6_mixed_flavors)."; \ + flavors="$$flavors $(ipv6_mixed_flavors)"; \ + else \ + echo "Skipping IPv6 mixed flavors:" \ + "$(ipv6_mixed_flavors)."; \ + skip_flavors="$$skip_flavors $(ipv6_mixed_flavors)"; \ + fi; \ + fi; \ + for f in $$skip_flavors; do \ echo "SKIP: $$f"; \ done; \ - for f in $(flavors); do \ + for f in $$flavors; do \ $(SHELL) "$(top_srcdir)/test-driver" --test-name "$$f" \ --log-file "$(TEST_NETWORK_ALL_LOG_DIR)/$$f.log" \ --trs-file "$(TEST_NETWORK_ALL_LOG_DIR)/$$f.trs" \

1 0

[tor/master] Makefile: Add an IPv6 mixed chutney network
by teor＠torproject.org 19 Feb '20

19 Feb '20

commit da280a430906349fd697860fbe326de86eccd36d Author: teor <teor(a)torproject.org> Date: Mon Feb 17 14:50:24 2020 +1000 Makefile: Add an IPv6 mixed chutney network This network is used in make test-network-all and test-network-ipv6. Closes 33334. --- Makefile.am | 6 ++++-- changes/ticket33334 | 5 +++++ src/test/include.am | 3 +++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Makefile.am b/Makefile.am index 7e57570a0..028488565 100644 --- a/Makefile.am +++ b/Makefile.am @@ -317,7 +317,8 @@ test-network-all: @$(MAKE) test-network-run \ ipv4_flavors="$(TEST_CHUTNEY_FLAVORS)" \ mixed_flavors="$(TEST_CHUTNEY_FLAVORS_MIXED)" \ - ipv6_flavors="$(TEST_CHUTNEY_FLAVORS_IPV6)" + ipv6_flavors="$(TEST_CHUTNEY_FLAVORS_IPV6)" \ + ipv6_mixed_flavors="$(TEST_CHUTNEY_FLAVORS_IPV6_MIXED)" @$(MAKE) test-network-results # Run IPv4 and mixed tests using automake's test-driver @@ -338,7 +339,8 @@ test-network-ipv6: @$(MAKE) test-network-mkdir @$(MAKE) test-network-clean @$(MAKE) test-network-run \ - ipv6_flavors="$(TEST_CHUTNEY_FLAVORS_IPV6)" + ipv6_flavors="$(TEST_CHUTNEY_FLAVORS_IPV6)" \ + ipv6_mixed_flavors="$(TEST_CHUTNEY_FLAVORS_IPV6_MIXED)" @$(MAKE) test-network-results # Make the test network log directory, if it does not exist diff --git a/changes/ticket33334 b/changes/ticket33334 new file mode 100644 index 000000000..ada3cb284 --- /dev/null +++ b/changes/ticket33334 @@ -0,0 +1,5 @@ + o Testing: + - Test v3 onion services to tor's mixed IPv4 chutney network. And add a + mixed IPv6 chutney network. These networks are used in the + test-network-all, test-network-ipv4, and test-network-ipv6 make targets. + Closes ticket 33334. diff --git a/src/test/include.am b/src/test/include.am index 9b5376d87..302771374 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -79,6 +79,9 @@ TEST_CHUTNEY_FLAVORS_IPV6 = bridges+ipv6-min ipv6-exit-min hs-v23-ipv6-md \ # only run if we can find a stable (or simply another) version of tor TEST_CHUTNEY_FLAVORS_MIXED = mixed+hs-v23 +# only run if IPv6 and mixed networks are run +TEST_CHUTNEY_FLAVORS_IPV6_MIXED = mixed+hs-v23-ipv6 + ### This is a lovely feature, but it requires automake >= 1.12, and Tor ### doesn't require that yet. ###

1 0

[tor/master] Makefile: Tweak the test-network* chuttney networks
by teor＠torproject.org 19 Feb '20

19 Feb '20

commit 6e76570ede710dffcaef798e1dd537cd78480861 Author: teor <teor(a)torproject.org> Date: Wed Feb 19 09:29:02 2020 +1000 Makefile: Tweak the test-network* chuttney networks In "make test-network", add tests for single onion services v2 and v3. In "make test-network-all", test onion services v2 and v3 in the same network. Part of 33334. --- src/test/include.am | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/include.am b/src/test/include.am index 302771374..9eb8dc2ab 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -66,11 +66,10 @@ TESTS += src/test/test src/test/test-slow src/test/test-memwipe \ # this test only uses IPv4 TEST_CHUTNEY_FLAVOR_QUICK = bridges+hs-v23 # only run if we can ping6 ::1 (localhost) -TEST_CHUTNEY_FLAVOR_QUICK_IPV6 = client-ipv6-only-md +TEST_CHUTNEY_FLAVOR_QUICK_IPV6 = single-onion-v23-ipv6-md # run a basic set of tests, which only use IPv4 -TEST_CHUTNEY_FLAVORS = basic-min bridges-min hs-v2-min hs-v3-min \ - single-onion-v23 +TEST_CHUTNEY_FLAVORS = basic-min bridges-min hs-v23-min single-onion-v23 # only run if we can ping ::1 (localhost) TEST_CHUTNEY_FLAVORS_IPV6 = bridges+ipv6-min ipv6-exit-min hs-v23-ipv6-md \

1 0

[tor/master] Makefile: Silence some submake logs in test-network
by teor＠torproject.org 19 Feb '20

19 Feb '20

commit aebae8225b910262b37c6b7a24bc144b0823a6e0 Author: teor <teor(a)torproject.org> Date: Mon Feb 17 22:03:09 2020 +1000 Makefile: Silence some submake logs in test-network These logs split up the chutney SKIP, PASS, and FAIL messages, and they don't actually contain any useful information. Cleanup after 33334. --- Makefile.am | 46 ++++++++++++++++++++-------------------------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/Makefile.am b/Makefile.am index 028488565..b9bae2395 100644 --- a/Makefile.am +++ b/Makefile.am @@ -297,61 +297,57 @@ need-chutney-path: # Run some basic tests using automake's test-driver .PHONY: test-network -# Make prints all these recursive invocations by default, but those extra lines -# break up the chutney flavor statuses -.SILENT: test-network +# Hide directory path logs from submakes using $(MAKE) -s test-network: - @$(MAKE) test-network-mkdir - @$(MAKE) test-network-clean - @$(MAKE) test-network-run \ + @$(MAKE) -s test-network-mkdir + @$(MAKE) -s test-network-clean + @$(MAKE) -s test-network-run \ ipv4_flavors="$(TEST_CHUTNEY_FLAVOR_QUICK)" \ ipv6_flavors="$(TEST_CHUTNEY_FLAVOR_QUICK_IPV6)" - @$(MAKE) test-network-results + @$(MAKE) -s test-network-results # Run all available tests using automake's test-driver .PHONY: test-network-all -.SILENT: test-network-all +# Hide directory path logs from submakes using $(MAKE) -s test-network-all: - @$(MAKE) test-network-mkdir - @$(MAKE) test-network-clean - @$(MAKE) test-network-run \ + @$(MAKE) -s test-network-mkdir + @$(MAKE) -s test-network-clean + @$(MAKE) -s test-network-run \ ipv4_flavors="$(TEST_CHUTNEY_FLAVORS)" \ mixed_flavors="$(TEST_CHUTNEY_FLAVORS_MIXED)" \ ipv6_flavors="$(TEST_CHUTNEY_FLAVORS_IPV6)" \ ipv6_mixed_flavors="$(TEST_CHUTNEY_FLAVORS_IPV6_MIXED)" - @$(MAKE) test-network-results + @$(MAKE) -s test-network-results # Run IPv4 and mixed tests using automake's test-driver .PHONY: test-network-ipv4 -.SILENT: test-network-ipv4 +# Hide directory path logs from submakes using $(MAKE) -s test-network-ipv4: - @$(MAKE) test-network-mkdir - @$(MAKE) test-network-clean - @$(MAKE) test-network-run \ + @$(MAKE) -s test-network-mkdir + @$(MAKE) -s test-network-clean + @$(MAKE) -s test-network-run \ ipv4_flavors="$(TEST_CHUTNEY_FLAVORS)" \ mixed_flavors="$(TEST_CHUTNEY_FLAVORS_MIXED)" - @$(MAKE) test-network-results + @$(MAKE) -s test-network-results # Run IPv6 tests using automake's test-driver .PHONY: test-network-ipv6 -.SILENT: test-network-ipv6 +# Hide directory path logs from submakes using $(MAKE) -s test-network-ipv6: - @$(MAKE) test-network-mkdir - @$(MAKE) test-network-clean - @$(MAKE) test-network-run \ + @$(MAKE) -s test-network-mkdir + @$(MAKE) -s test-network-clean + @$(MAKE) -s test-network-run \ ipv6_flavors="$(TEST_CHUTNEY_FLAVORS_IPV6)" \ ipv6_mixed_flavors="$(TEST_CHUTNEY_FLAVORS_IPV6_MIXED)" - @$(MAKE) test-network-results + @$(MAKE) -s test-network-results # Make the test network log directory, if it does not exist .PHONY: test-network-mkdir -.SILENT: test-network-mkdir test-network-mkdir: @mkdir -p "$(TEST_NETWORK_ALL_LOG_DIR)" # Clean the test network log directory .PHONY: test-network-clean -# We don't silence clean: we want to see the rm command # We need to remove all matching files, so we can't quote the glob part of the # rm arguments test-network-clean: @@ -375,7 +371,6 @@ test-network-clean: # Run the IPv6 mixed tests in $(ipv6_mixed_flavors), if IPv6 and mixed are run # - see above for details about IPv6 and mixed .PHONY: test-network-run -.SILENT: test-network-run # We need the word splitting in the "for" lines, so we can't quote # $(skip_flavors) or $(flavors) test-network-run: need-chutney-path test-driver $(TESTING_TOR_BINARY) src/tools/tor-gencert @@ -454,7 +449,6 @@ test-network-run: need-chutney-path test-driver $(TESTING_TOR_BINARY) src/tools/ # (otherwise, warnings go to the logs, and people don't see them unless # there is a network failure) .PHONY: test-network-results -.SILENT: test-network-results # We need to grep all matching files, so we can't quote the glob part of the # grep arguments test-network-results:

1 0

[tor/master] Merge remote-tracking branch 'tor-github/pr/1744'
by teor＠torproject.org 19 Feb '20

19 Feb '20

commit 4dd594d33cf9821e1e8c2d9f92f0e0035173bf19 Merge: f0561861e 6e76570ed Author: teor <teor(a)torproject.org> Date: Wed Feb 19 10:09:29 2020 +1000 Merge remote-tracking branch 'tor-github/pr/1744' Makefile.am | 181 ++++++++++++++++++++++++++++------------------------ changes/ticket33334 | 5 ++ src/test/include.am | 12 ++-- 3 files changed, 108 insertions(+), 90 deletions(-)

1 0

[snowflake/master] Close internal Pipes in websocketconn.Conn Close.
by dcf＠torproject.org 18 Feb '20

18 Feb '20

commit 380b133155ad725126bc418d0e66b3c550b4c555 Author: David Fifield <david(a)bamsoftware.com> Date: Tue Feb 18 14:10:47 2020 -0700 Close internal Pipes in websocketconn.Conn Close. Unless something externally called Write after Close, the writeLoop(ws, pr2) goroutine would run forever, because nothing would ever close pw2/pr2. https://bugs.torproject.org/33367#comment:4 --- common/websocketconn/websocketconn.go | 2 ++ common/websocketconn/websocketconn_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/common/websocketconn/websocketconn.go b/common/websocketconn/websocketconn.go index 46bb977..c745522 100644 --- a/common/websocketconn/websocketconn.go +++ b/common/websocketconn/websocketconn.go @@ -24,6 +24,8 @@ func (conn *Conn) Write(b []byte) (n int, err error) { } func (conn *Conn) Close() error { + conn.Reader.(*io.PipeReader).Close() + conn.Writer.(*io.PipeWriter).Close() // Ignore any error in trying to write a Close frame. _ = conn.Conn.WriteControl(websocket.CloseMessage, []byte{}, time.Now().Add(time.Second)) return conn.Conn.Close() diff --git a/common/websocketconn/websocketconn_test.go b/common/websocketconn/websocketconn_test.go index ad6f100..92774d4 100644 --- a/common/websocketconn/websocketconn_test.go +++ b/common/websocketconn/websocketconn_test.go @@ -233,3 +233,31 @@ func TestConcurrentWrite(t *testing.T) { t.Fatalf("Write: %v", err) } } + +// Test that Read and Write methods return errors after Close. +func TestClose(t *testing.T) { + s, c, err := connPair() + if err != nil { + t.Fatal(err) + } + defer c.Close() + + err = s.Close() + if err != nil { + t.Fatal(err) + } + + var buf [10]byte + n, err := s.Read(buf[:]) + if n != 0 || err == nil { + t.Fatalf("Read after Close returned (%v, %v), expected (%v, non-nil)", n, err, 0) + } + + _, err = s.Write([]byte{1, 2, 3}) + // Here we break the abstraction a little and look for a specific error, + // io.ErrClosedPipe. This is because we know the Conn uses an io.Pipe + // internally. + if err != io.ErrClosedPipe { + t.Fatalf("Write after Close returned %v, expected %v", err, io.ErrClosedPipe) + } +}

1 0

[tor/master] Merge branch 'tor-github/pr/1746'
by dgoulet＠torproject.org 18 Feb '20

18 Feb '20

commit f0561861e306e9c9701d7ae35cf6a9158027b222 Merge: 17724a7cd 9a68eca3a Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Feb 18 13:57:03 2020 -0500 Merge branch 'tor-github/pr/1746' changes/ticket33349 | 4 +++ src/core/or/scheduler.c | 75 +++++++++++++++++++++++--------------------- src/core/or/scheduler_kist.c | 7 +++++ 3 files changed, 50 insertions(+), 36 deletions(-)

1 0

[tor/master] Small changes to scheduler comments and state changes
by dgoulet＠torproject.org 18 Feb '20

18 Feb '20

commit 9a68eca3a7044c549bb50521d13c6d7a9e51a6d3 Author: Steven Engler <sengler(a)uwaterloo.ca> Date: Mon Feb 17 11:38:00 2020 -0500 Small changes to scheduler comments and state changes Tries to make some of the comments in scheduler.c easier to follow, and simplifies a couple of the scheduler channel state changes. --- changes/ticket33349 | 4 +++ src/core/or/scheduler.c | 75 +++++++++++++++++++++++--------------------- src/core/or/scheduler_kist.c | 7 +++++ 3 files changed, 50 insertions(+), 36 deletions(-) diff --git a/changes/ticket33349 b/changes/ticket33349 new file mode 100644 index 000000000..0458a72c8 --- /dev/null +++ b/changes/ticket33349 @@ -0,0 +1,4 @@ + o Code simplification and refactoring: + - Updated comments in 'scheduler.c' to reflect old code changes, + and simplified the scheduler channel state change code. Closes + ticket 33349. diff --git a/src/core/or/scheduler.c b/src/core/or/scheduler.c index cd9aa5464..6633ccfe1 100644 --- a/src/core/or/scheduler.c +++ b/src/core/or/scheduler.c @@ -502,7 +502,12 @@ scheduler_free_all(void) the_scheduler = NULL; } -/** Mark a channel as no longer ready to accept writes. */ +/** Mark a channel as no longer ready to accept writes. + * + * Possible state changes: + * - SCHED_CHAN_PENDING -> SCHED_CHAN_WAITING_TO_WRITE + * - SCHED_CHAN_WAITING_FOR_CELLS -> SCHED_CHAN_IDLE + */ MOCK_IMPL(void, scheduler_channel_doesnt_want_writes,(channel_t *chan)) { @@ -513,31 +518,32 @@ scheduler_channel_doesnt_want_writes,(channel_t *chan)) return; } - /* If it's already in pending, we can put it in waiting_to_write */ if (chan->scheduler_state == SCHED_CHAN_PENDING) { /* - * It's in channels_pending, so it shouldn't be in any of - * the other lists. It can't write any more, so it goes to - * channels_waiting_to_write. + * It has cells but no longer can write, so it becomes + * SCHED_CHAN_WAITING_TO_WRITE. It's in channels_pending, so we + * should remove it from the list. */ smartlist_pqueue_remove(channels_pending, scheduler_compare_channels, offsetof(channel_t, sched_heap_idx), chan); scheduler_set_channel_state(chan, SCHED_CHAN_WAITING_TO_WRITE); - } else { + } else if (chan->scheduler_state == SCHED_CHAN_WAITING_FOR_CELLS) { /* - * It's not in pending, so it can't become waiting_to_write; it's - * either not in any of the lists (nothing to do) or it's already in - * waiting_for_cells (remove it, can't write any more). + * It does not have cells and no longer can write, so it becomes + * SCHED_CHAN_IDLE. */ - if (chan->scheduler_state == SCHED_CHAN_WAITING_FOR_CELLS) { - scheduler_set_channel_state(chan, SCHED_CHAN_IDLE); - } + scheduler_set_channel_state(chan, SCHED_CHAN_IDLE); } } -/** Mark a channel as having waiting cells. */ +/** Mark a channel as having waiting cells. + * + * Possible state changes: + * - SCHED_CHAN_WAITING_FOR_CELLS -> SCHED_CHAN_PENDING + * - SCHED_CHAN_IDLE -> SCHED_CHAN_WAITING_TO_WRITE + */ MOCK_IMPL(void, scheduler_channel_has_waiting_cells,(channel_t *chan)) { @@ -548,12 +554,11 @@ scheduler_channel_has_waiting_cells,(channel_t *chan)) return; } - /* First, check if it's also writeable */ if (chan->scheduler_state == SCHED_CHAN_WAITING_FOR_CELLS) { /* - * It's in channels_waiting_for_cells, so it shouldn't be in any of - * the other lists. It has waiting cells now, so it goes to - * channels_pending. + * It is able to write and now has cells, so it becomes + * SCHED_CHAN_PENDING. It must be added to the channels_pending + * list. */ scheduler_set_channel_state(chan, SCHED_CHAN_PENDING); if (!SCHED_BUG(chan->sched_heap_idx != -1, chan)) { @@ -565,16 +570,12 @@ scheduler_channel_has_waiting_cells,(channel_t *chan)) /* If we made a channel pending, we potentially have scheduling work to * do. */ the_scheduler->schedule(); - } else { + } else if (chan->scheduler_state == SCHED_CHAN_IDLE) { /* - * It's not in waiting_for_cells, so it can't become pending; it's - * either not in any of the lists (we add it to waiting_to_write) - * or it's already in waiting_to_write or pending (we do nothing) + * It is not able to write but now has cells, so it becomes + * SCHED_CHAN_WAITING_TO_WRITE. */ - if (!(chan->scheduler_state == SCHED_CHAN_WAITING_TO_WRITE || - chan->scheduler_state == SCHED_CHAN_PENDING)) { - scheduler_set_channel_state(chan, SCHED_CHAN_WAITING_TO_WRITE); - } + scheduler_set_channel_state(chan, SCHED_CHAN_WAITING_TO_WRITE); } } @@ -662,8 +663,12 @@ scheduler_release_channel,(channel_t *chan)) scheduler_set_channel_state(chan, SCHED_CHAN_IDLE); } -/** Mark a channel as ready to accept writes */ - +/** Mark a channel as ready to accept writes. + * Possible state changes: + * + * - SCHED_CHAN_WAITING_TO_WRITE -> SCHED_CHAN_PENDING + * - SCHED_CHAN_IDLE -> SCHED_CHAN_WAITING_FOR_CELLS + */ void scheduler_channel_wants_writes(channel_t *chan) { @@ -674,10 +679,11 @@ scheduler_channel_wants_writes(channel_t *chan) return; } - /* If it's already in waiting_to_write, we can put it in pending */ if (chan->scheduler_state == SCHED_CHAN_WAITING_TO_WRITE) { /* - * It can write now, so it goes to channels_pending. + * It has cells and can now write, so it becomes + * SCHED_CHAN_PENDING. It must be added to the channels_pending + * list. */ scheduler_set_channel_state(chan, SCHED_CHAN_PENDING); if (!SCHED_BUG(chan->sched_heap_idx != -1, chan)) { @@ -688,15 +694,12 @@ scheduler_channel_wants_writes(channel_t *chan) } /* We just made a channel pending, we have scheduling work to do. */ the_scheduler->schedule(); - } else { + } else if (chan->scheduler_state == SCHED_CHAN_IDLE) { /* - * It's not in SCHED_CHAN_WAITING_TO_WRITE, so it can't become pending; - * it's either idle and goes to WAITING_FOR_CELLS, or it's a no-op. + * It does not have cells but can now write, so it becomes + * SCHED_CHAN_WAITING_FOR_CELLS. */ - if (!(chan->scheduler_state == SCHED_CHAN_WAITING_FOR_CELLS || - chan->scheduler_state == SCHED_CHAN_PENDING)) { - scheduler_set_channel_state(chan, SCHED_CHAN_WAITING_FOR_CELLS); - } + scheduler_set_channel_state(chan, SCHED_CHAN_WAITING_FOR_CELLS); } } diff --git a/src/core/or/scheduler_kist.c b/src/core/or/scheduler_kist.c index e56942be0..b6f584645 100644 --- a/src/core/or/scheduler_kist.c +++ b/src/core/or/scheduler_kist.c @@ -463,6 +463,13 @@ MOCK_IMPL(void, channel_write_to_kernel, (channel_t *chan)) log_debug(LD_SCHED, "Writing %lu bytes to kernel for chan %" PRIu64, (unsigned long)channel_outbuf_length(chan), chan->global_identifier); + /* Note that 'connection_handle_write()' may change the scheduler state of + * the channel during the scheduling loop with + * 'connection_or_flushed_some()' -> 'scheduler_channel_wants_writes()'. + * This side-effect will only occur if the channel is currently in the + * 'SCHED_CHAN_WAITING_TO_WRITE' or 'SCHED_CHAN_IDLE' states, which KIST + * rarely uses, so it should be fine unless KIST begins using these states + * in the future. */ connection_handle_write(TO_CONN(BASE_CHAN_TO_TLS(chan)->conn), 0); }

1 0