Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arg_readers Process tests broken on windows #20

Closed
vtjnash opened this issue Feb 10, 2021 · 9 comments · Fixed by JuliaIO/Tar.jl#115
Closed

arg_readers Process tests broken on windows #20

vtjnash opened this issue Feb 10, 2021 · 9 comments · Fixed by JuliaIO/Tar.jl#115

Comments

@vtjnash
Copy link

vtjnash commented Feb 10, 2021

When run properly (for example, JuliaLang/julia#39544) the tests on Windows fail, as they neglect to properly drain the process output pipe, as is mandatory for correct and successful operation on all operating systems (though this condition is currently only tested for and enforced on Windows, unless we merge JuliaLang/julia#39574).

An example test failure may either look like the following, or simply will deadlock the process until CI time expires:

Error in testset Tar:
Error During Test at C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.7\Tar\test\runtests.jl:492
  Got exception outside of a @test
  failed process: Process(`cat 'C:\Windows\TEMP\jl_9FC0.tmp'`, ProcessExited(1)) [1]
  
  Stacktrace:
    [1] pipeline_error
      @ .\process.jl:526 [inlined]
    [2] open(::Main.Test30Main_Tar.var"#118#136", ::Cmd; kwargs::Base.Iterators.Pairs{Union{}, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
      @ Base .\process.jl:401
    [3] open
      @ .\process.jl:392 [inlined]
    [4] #10
      @ C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.7\ArgTools\src\ArgTools.jl:187 [inlined]
    [5] (::Main.Test30Main_Tar.var"#114#132"{String})(tar::ArgTools.var"#10#20"{String})
      @ Main.Test30Main_Tar C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.7\Tar\test\runtests.jl:512
    [6] arg_readers(body::Main.Test30Main_Tar.var"#114#132"{String}, path::String, type::Type)
      @ ArgTools C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.7\ArgTools\src\ArgTools.jl:230
    [7] arg_readers(body::Function, path::String)
      @ ArgTools C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.7\ArgTools\src\ArgTools.jl:228
    [8] macro expansion
      @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.7\Tar\test\runtests.jl:493 [inlined]
    [9] macro expansion
      @ C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.7\Test\src\Test.jl:1152 [inlined]
   [10] macro expansion
      @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.7\Tar\test\runtests.jl:493 [inlined]
   [11] macro expansion
      @ C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.7\Test\src\Test.jl:1152 [inlined]
   [12] top-level scope
      @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.7\Tar\test\runtests.jl:485
   [13] include
      @ .\Base.jl:386 [inlined]
   [14] macro expansion
      @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:24 [inlined]
   [15] macro expansion
      @ C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.7\Test\src\Test.jl:1152 [inlined]
   [16] macro expansion
      @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:23 [inlined]
   [17] macro expansion
      @ .\timing.jl:356 [inlined]
   [18] runtests(name::String, path::String, isolate::Bool; seed::UInt128)
      @ Main C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:21
   [19] (::Distributed.var"#106#108"{Distributed.CallMsg{:call_fetch}})()
      @ Distributed C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.7\Distributed\src\process_messages.jl:278
   [20] run_work_thunk(thunk::Distributed.var"#106#108"{Distributed.CallMsg{:call_fetch}}, print_error::Bool)
      @ Distributed C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.7\Distributed\src\process_messages.jl:63
   [21] macro expansion
      @ C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.7\Distributed\src\process_messages.jl:278 [inlined]
   [22] (::Distributed.var"#105#107"{Distributed.CallMsg{:call_fetch}, Distributed.MsgHeader, Sockets.TCPSocket})()
      @ Distributed .\task.jl:406
@vtjnash
Copy link
Author

vtjnash commented Mar 11, 2021

Bump. Should this be considered an issue with Tar.jl mis-using the package, or with ArgTools mis-handling the difference between streams and files? This is causing CI reliability problems, so we may need to hear from the designers here on what they intended.

@StefanKarpinski
Copy link
Member

Could probably add code to drain the process. Would that fix the problem?

@StefanKarpinski
Copy link
Member

Of course, if the operation in question needs to drain the input in order not to error, then it should do that. Which suggests changing Tar instead.

@vtjnash
Copy link
Author

vtjnash commented Mar 22, 2021

Yes, those are likely the two main possible courses of action. I think draining the output is the most robust solution for Cmd, in isolation, but then may raise questions about how to handle Process or Pipe, which suggests possibly Tar should be changed instead. (and doesn't rule out just making both changes either)

@vtjnash
Copy link
Author

vtjnash commented Apr 16, 2021

bump

@StefanKarpinski
Copy link
Member

I think this PR JuliaIO/Tar.jl#114 might fix this issue but it's hard to tell since I can't reproduce the problem in Tar.jl even using Julia nightly.

@vtjnash
Copy link
Author

vtjnash commented Jun 2, 2021

It should be tested on JuliaLang/julia#39544, not master

@StefanKarpinski
Copy link
Member

Will that fail on all platforms or do I need to test on a specific one?

@StefanKarpinski
Copy link
Member

Should be fixed on the latest Tar.jl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants