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

GAP command Process hangs after IO_Popen3 is used on macOS #902

Open
mohamed-barakat opened this issue Jun 17, 2023 · 6 comments
Open

GAP command Process hangs after IO_Popen3 is used on macOS #902

mohamed-barakat opened this issue Jun 17, 2023 · 6 comments

Comments

@mohamed-barakat
Copy link
Contributor

mohamed-barakat commented Jun 17, 2023

On macOS

$ julia
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.9.1 (2023-06-07)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> using GAP
 ┌───────┐   GAP 4.12.2 of 2022-12-18
 │  GAP  │   https://www.gap-system.org
 └───────┘   Architecture: aarch64-apple-darwin20-julia1.9-64-kv8
 Configuration:  gmp 6.2.1, Julia GC, Julia 1.9.1, readline
 Loading the library and packages ...
 Packages:   AClib 1.3.2, Alnuth 3.2.1, AtlasRep 2.1.6, AutPGrp 1.11, CRISP 1.4.6, Cryst 4.1.25, CrystCat 1.1.10, CTblLib 1.3.4, FactInt 1.6.3, FGA 1.4.0, Forms 1.2.9,
             GAPDoc 1.6.6, genss 1.6.8, IO 4.8.1, IRREDSOL 1.4.4, JuliaInterface 0.9.6, LAGUNA 3.9.5, orb 4.9.0, Polenta 1.3.10, Polycyclic 2.16, PrimGrp 3.4.3,
             RadiRoot 2.9, recog 1.4.2, ResClasses 4.7.3, SmallGrp 1.5.1, Sophus 1.27, SpinSym 1.5.2, TomLib 1.2.9, TransGrp 3.6.3, utils 0.81
 Try '??help' for help. See also '?copyright', '?cite' and '?authors'

julia> GAP.Globals.str = g""; GAP.Globals.Process( GAP.Globals.DirectoryCurrent(), GAP.Globals.Filename( GAP.Globals.DirectoriesSystemPrograms(), g"sh" ), GAP.Globals.InputTextUser(), GAP.Globals.OutputTextString( GAP.Globals.str, true ), GapObj( [ g"-c", g"echo 2" ] ) ); GAP.Globals.str
GAP: "2\n"

julia> GAP.Globals.str = g""; GAP.Globals.Process( GAP.Globals.DirectoryCurrent(), GAP.Globals.Filename( GAP.Globals.DirectoriesSystemPrograms(), g"sh" ), GAP.Globals.InputTextUser(), GAP.Globals.OutputTextString( GAP.Globals.str, true ), GapObj( [ g"-c", g"echo 2" ] ) ); GAP.Globals.str
GAP: "2\n"

julia> GAP.Globals.IO_Popen3( GAP.Globals.Filename( GAP.Globals.DirectoriesSystemPrograms( ), g"env" ), GapObj( [ ] ) )

julia> GAP.Globals.str = g""; GAP.Globals.Process( GAP.Globals.DirectoryCurrent(), GAP.Globals.Filename( GAP.Globals.DirectoriesSystemPrograms(), g"sh" ), GAP.Globals.InputTextUser(), GAP.Globals.OutputTextString( GAP.Globals.str, true ), GapObj( [ g"-c", g"echo 2" ] ) ); GAP.Globals.str
<hangs>

On Ubuntu

$ julia
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.9.1 (2023-06-07)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> using GAP
 ┌───────┐   GAP 4.12.2 of 2022-12-18
 │  GAP  │   https://www.gap-system.org
 └───────┘   Architecture: aarch64-apple-darwin20-julia1.9-64-kv8
 Configuration:  gmp 6.2.1, Julia GC, Julia 1.9.1, readline
 Loading the library and packages ...
 Packages:   AClib 1.3.2, Alnuth 3.2.1, AtlasRep 2.1.6, AutPGrp 1.11, CRISP 1.4.6, Cryst 4.1.25, CrystCat 1.1.10, CTblLib 1.3.4, FactInt 1.6.3, FGA 1.4.0, Forms 1.2.9,
             GAPDoc 1.6.6, genss 1.6.8, IO 4.8.1, IRREDSOL 1.4.4, JuliaInterface 0.9.6, LAGUNA 3.9.5, orb 4.9.0, Polenta 1.3.10, Polycyclic 2.16, PrimGrp 3.4.3,
             RadiRoot 2.9, recog 1.4.2, ResClasses 4.7.3, SmallGrp 1.5.1, Sophus 1.27, SpinSym 1.5.2, TomLib 1.2.9, TransGrp 3.6.3, utils 0.81
 Try '??help' for help. See also '?copyright', '?cite' and '?authors'

julia> GAP.Globals.str = g""; GAP.Globals.Process( GAP.Globals.DirectoryCurrent(), GAP.Globals.Filename( GAP.Globals.DirectoriesSystemPrograms(), g"sh" ), GAP.Globals.InputTextUser(), GAP.Globals.OutputTextString( GAP.Globals.str, true ), GapObj( [ g"-c", g"echo 2" ] ) ); GAP.Globals.str
GAP: "2\n"

julia> GAP.Globals.str = g""; GAP.Globals.Process( GAP.Globals.DirectoryCurrent(), GAP.Globals.Filename( GAP.Globals.DirectoriesSystemPrograms(), g"sh" ), GAP.Globals.InputTextUser(), GAP.Globals.OutputTextString( GAP.Globals.str, true ), GapObj( [ g"-c", g"echo 2" ] ) ); GAP.Globals.str
GAP: "2\n"

julia> GAP.Globals.IO_Popen3( GAP.Globals.Filename( GAP.Globals.DirectoriesSystemPrograms( ), g"env" ), GapObj( [ ] ) )

julia> GAP.Globals.str = g""; GAP.Globals.Process( GAP.Globals.DirectoryCurrent(), GAP.Globals.Filename( GAP.Globals.DirectoriesSystemPrograms(), g"sh" ), GAP.Globals.InputTextUser(), GAP.Globals.OutputTextString( GAP.Globals.str, true ), GapObj( [ g"-c", g"echo 2" ] ) ); GAP.Globals.str
GAP: "2\n"

julia>
@fingolfin
Copy link
Member

I have not analyzed this in detail yet, but in a nutshell, on the long term, all uses of low-level IO functions like IO_Popen3 or IO_fork should be removed resp. replaced with better functions that allow a higher level abstraction.

Thing is, handling child processes on POSIX is quite delicate. The central program must be aware of them and mange them. That caused issues in pure GAP in the past, when IO did not do that and instead just managed "rogue" child processes of its own. It took quite some effort to resolve that, by inventing a way for the GAP kernel and the IO kernel extension to communicate and coordinate the child processes started by either one.

Now we are running GAP and IO from Julia, so they ought to coordinate with Julia.... For GAP, this is already implemented for Process (via a Julia function GAP_ExecuteProcess, a replacement for GAP's ExecuteProcess). But the new implementation does not (yet?!?) coordinate with IO... hence the hang

As a workaround, you can do this:

GAP.use_orig_ExecuteProcess[]=true

Maybe we should make this the default again for the time being, as I suspect this will also resolve #905 (CC @ThomasBreuer).

But on the long term, it would be best if any use of IO_Popen3 (and more importantly; IO_fork) could be replaced by calls to a generic enough API so that we can implement it with IO in classic GAP, and with calls to corresponding Julia functions in GAP.jl.

@mohamed-barakat
Copy link
Contributor Author

Thank you very much for the feedback.

@mohamed-barakat
Copy link
Contributor Author

I heavily rely on the IO package as one of the possible ways to communicate with external programs. Switching to GAP's built-in IO capabilities will be quite some work, I guess.

@fingolfin
Copy link
Member

AFAIK the existing facilities in GAP are too limited in their capabilities and also too slow for you, isn't that right?

So I am not at all suggesting you should use those, but rather we should have a higher level abstraction for what you actually need with multiple implementations (one of them would still use the IO package, and another could use Julia code)

@mohamed-barakat
Copy link
Contributor Author

mohamed-barakat commented Jun 27, 2023

So I am not at all suggesting you should use those, but rather we should have a higher level abstraction for what you actually need with multiple implementations (one of them would still use the IO package, and another could use Julia code)

The higher abstraction is indeed provided by the package HomalgToCAS, where either IO_ForHomalg, SinglularForHomalg, MapleForHomalg, OscarForHomalg, ... could be used as the "physical" connection.

@mohamed-barakat
Copy link
Contributor Author

AFAIK the existing facilities in GAP are too limited in their capabilities and also too slow for you, isn't that right?

Correct.

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

No branches or pull requests

2 participants