Project

General

Profile

Actions

Bug #12910

closed

TestFileUtils#test_chown_R_force might get stuck when process has no supplementary groups

Added by vo.x (Vit Ondruch) about 5 years ago. Updated almost 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
ruby 2.4.0dev (2016-11-07 trunk 56664) [x86_64-linux]
[ruby-core:78053]

Description

Testing build of Ruby using systemd-nspawn, the test suite gets always stuck when executing TestFileUtils#test_chown_R_force. Pressing Ctrl+C, it proceeds with following backtrace:

  1) Failure:
TestFileUtils#test_chown_R_force [/builddir/build/BUILD/ruby-2.4.0-r56664/test/fileutils/test_fileutils.rb:1261]:
exceptions on 2 threads:
#<Thread:0x0055600e846cf0@/builddir/build/BUILD/ruby-2.4.0-r56664/test/fileutils/test_fileutils.rb:18 sleep>:
/builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/test/unit/assertions.rb:762:in `value':  (Interrupt)
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/test/unit/assertions.rb:762:in `assert_join_threads'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/fileutils/test_fileutils.rb:23:in `block in assert_output_lines'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/fileutils/test_fileutils.rb:16:in `pipe'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/fileutils/test_fileutils.rb:16:in `assert_output_lines'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/fileutils/test_fileutils.rb:1261:in `test_chown_R_force'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/test/unit.rb:1029:in `run_test'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/minitest/unit.rb:1269:in `run'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/test/unit/testcase.rb:18:in `run'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/minitest/unit.rb:941:in `block in _run_suite'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/minitest/unit.rb:934:in `map'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/minitest/unit.rb:934:in `_run_suite'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/test/unit.rb:914:in `_run_suite'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/test/unit.rb:494:in `block in _run_suites'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/test/unit.rb:492:in `each'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/test/unit.rb:492:in `_run_suites'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/test/unit.rb:530:in `_run_suites'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/minitest/unit.rb:885:in `_run_anything'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/minitest/unit.rb:1096:in `run_tests'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/minitest/unit.rb:1083:in `block in _run'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/minitest/unit.rb:1082:in `each'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/minitest/unit.rb:1082:in `_run'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/minitest/unit.rb:1070:in `run'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/test/unit.rb:682:in `run'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/test/unit.rb:33:in `run'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/test/unit.rb:991:in `run'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/test/unit.rb:995:in `run'
    from ./test/runner.rb:40:in `<main>'
---
#<Thread:0x0055600e8469f8@/builddir/build/BUILD/ruby-2.4.0-r56664/test/fileutils/test_fileutils.rb:19 dead>:
/builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/minitest/unit.rb:201:in `assert': File group ownership of "tmp/dir" unexpected:
 Expected: <>
   Actual: <135>
 (MiniTest::Assertion)
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/lib/test/unit/assertions.rb:37:in `assert'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/fileutils/fileasserts.rb:95:in `assert_ownership_group'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/fileutils/test_fileutils.rb:1278:in `block (2 levels) in test_chown_R_force'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/fileutils/test_fileutils.rb:1273:in `each'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/fileutils/test_fileutils.rb:1273:in `block in test_chown_R_force'
    from /builddir/build/BUILD/ruby-2.4.0-r56664/test/fileutils/test_fileutils.rb:20:in `block (2 levels) in assert_output_lines'

And there are two issues with this:

  1. It should simply fail and not get stuck, no matter what.

  2. For some reason, the test expects there will be some supplementary groups available, but there are non in the container. The following patch makes the test suite pass, although it apparently doesn't test everything, since some tests expects more than one group:

$ git show
commit 43934fbbf16c7f19e07ae1de17fce6697fb137cc
Author: Vít Ondruch <vondruch@redhat.com>
Date:   Tue Nov 8 16:25:24 2016 +0100

    Use primary group as well as supplementary groups.

    I might happen in certain environments (systemd-nspawn) that process has
    no supplementary groups, but primary groups should be enough to pass most
    of the tests.

diff --git a/test/fileutils/test_fileutils.rb b/test/fileutils/test_fileutils.rb
index 0ff0aad..fa5b24d 100644
--- a/test/fileutils/test_fileutils.rb
+++ b/test/fileutils/test_fileutils.rb
@@ -142,7 +142,7 @@ def mymkdir(path)

   def setup
     @prevdir = Dir.pwd
-    @groups = Process.groups if have_file_perm?
+    @groups = [Process.gid, Process.groups].flatten if have_file_perm?
     tmproot = @tmproot = Dir.mktmpdir "fileutils"
     Dir.chdir tmproot
     my_rm_rf 'data'; mymkdir 'data'

Updated by vo.x (Vit Ondruch) about 5 years ago

although it apparently doesn't test everything, since some tests expects more than one group:

Which is actually not different from the current state, when I am testing this in chroot created by Mock.

Updated by vo.x (Vit Ondruch) about 5 years ago

For some reason, the test expects there will be some supplementary groups available, but there are non in the container. The following patch makes the test suite pass, although it apparently doesn't test everything, since some tests expects more than one group:

Here is the response of systemd-nspawn maintainer 1:

The getgroups(2) man page is pretty clear:

       It is unspecified  whether  the effective group ID of the
       calling process is included in the  returned  list.
       (Thus,  an  application should also call getegid(2)
       and add or remove the resulting value.)

I don't think there's any bug here.

So I think that the proposed patch should be applied.

Actions #3

Updated by nobu (Nobuyoshi Nakada) about 5 years ago

  • Status changed from Open to Closed

Applied in changeset r56884.


test_fileutils.rb: Use primary group too

  • test/fileutils/test_fileutils.rb (TestFileUtils#setup): Use primary group as well as supplementary groups. based on the patch by Vít Ondruch at [ruby-core:78053]. [Bug #12910]

It might happen in certain environments (systemd-nspawn) that
process has no supplementary groups, but primary groups should be
enough to pass most of the tests.

Updated by nagachika (Tomoyuki Chikanaga) about 5 years ago

  • Backport changed from 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN to 2.1: UNKNOWN, 2.2: REQUIRED, 2.3: REQUIRED

Updated by nagachika (Tomoyuki Chikanaga) about 5 years ago

  • Backport changed from 2.1: UNKNOWN, 2.2: REQUIRED, 2.3: REQUIRED to 2.1: UNKNOWN, 2.2: REQUIRED, 2.3: DONE

ruby_2_3 r56888 merged revision(s) 56884.

Updated by vo.x (Vit Ondruch) about 5 years ago

Thx for fixing this. But is there chance to improve my first point? I.e. why the test suite gets stuck? It should just fail. The assert apparently failed, so the test case should fail ....

Updated by nagachika (Tomoyuki Chikanaga) about 5 years ago

  • Backport changed from 2.1: UNKNOWN, 2.2: REQUIRED, 2.3: DONE to 2.1: UNKNOWN, 2.2: REQUIRED, 2.3: REQUIRED

Updated by nobu (Nobuyoshi Nakada) almost 5 years ago

  • Backport changed from 2.1: UNKNOWN, 2.2: REQUIRED, 2.3: REQUIRED to 2.1: REQUIRED, 2.2: REQUIRED, 2.3: REQUIRED

Updated by usa (Usaku NAKAMURA) almost 5 years ago

  • Backport changed from 2.1: REQUIRED, 2.2: REQUIRED, 2.3: REQUIRED to 2.1: REQUIRED, 2.2: DONE, 2.3: REQUIRED

ruby_2_2 r57223 merged revision(s) 56884,56892.

Updated by nagachika (Tomoyuki Chikanaga) almost 5 years ago

  • Backport changed from 2.1: REQUIRED, 2.2: DONE, 2.3: REQUIRED to 2.1: REQUIRED, 2.2: DONE, 2.3: DONE

ruby_2_3 r57566 merged revision(s) 56892.

Actions

Also available in: Atom PDF