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

Python binding hangs in forked process #535

Open
EmpireJones opened this issue Sep 22, 2022 · 7 comments
Open

Python binding hangs in forked process #535

EmpireJones opened this issue Sep 22, 2022 · 7 comments

Comments

@EmpireJones
Copy link

When using the Python bindings with multiprocessing (forking the process via a Pool, to parallelize the minify work), minify appears to hang.

In particular, this seems to happen when minify is imported via the main process, prior to forking the new processes.

A workaround is to ensure that the library doesn't get imported prior to forking (i.e., have the new forked processes perform the import).

I'm not sure if there's any easy library change to fix this, but I at least wanted to let others know of the workaround.

--

I'm using Python3.10.4; the installed go version is go1.17.1; Ubuntu 22.04.1.

@tdewolff
Copy link
Owner

Thank you for the feedback. The JavaScript binding has a very similar problem. I'm not sure if this is a bug in minify itself (I don't think so), or in the bindings themselves (maybe), or in the interaction with Go and Python/JavaScript. It is nonetheless very hard to debug, but since I have a lot more experience in Python than in JS, could you please post a small script that reproduces the issue?

@EmpireJones
Copy link
Author

EmpireJones commented Sep 23, 2022

I was able to reproduce it with the following code using python 3.10.4. I had to bump up the number_list size and html length to get it to occur, so you may need to tweak those based on the system.

import multiprocessing
import minify  # type: ignore


def minify_worker(number: int) -> None:
    minify.string('text/html', f"<p>{number}</p>"*10000)
    print(".", end="", flush=True)


if __name__ == "__main__":

    number_list = range(0, 1000)

    processing_pool = multiprocessing.Pool()
    processing_pool.map(minify_worker, number_list)

    print("Done!")

and the workaround is to move the import to the child process:

import multiprocessing


def minify_worker(number: int) -> None:
    import minify  # type: ignore
    minify.string('text/html', f"<p>{number}</p>"*10000)
    print(".", end="", flush=True)


if __name__ == "__main__":

    number_list = range(0, 1000)

    processing_pool = multiprocessing.Pool()
    processing_pool.map(minify_worker, number_list)

    print("Done!")

@tdewolff
Copy link
Owner

Thanks Kevin, this is hanging here as well. I'll see what I can do...

@tdewolff
Copy link
Owner

tdewolff commented Sep 23, 2022

Update: it seems to be resolved when running GOGC=off python test.py, where test.py is the first script you posted.

Calling runtime.GC() always hangs in this case. This even happens when multiprocessing.Pool(processes=1) which makes sure there is only one thread running. The difference is that:

  • without multiprocessing, main PID == thread PID
  • with multiprocessing: main PID != thread PID

where main PID is unix.Getpid() inside the special init() method, and thread PID is unix.Getpid() inside the string() method of the library.

runtime.LockOSThread() seems to have no effect.

@EmpireJones
Copy link
Author

EmpireJones commented Sep 23, 2022

An excerpt from https://stackoverflow.com/questions/28370646/how-do-i-fork-a-go-process:

Note that fork() has been invented at the time when no threads were used at all, and a process had always had just a single thread of execution in it, and hence forking it was safe. With Go, the situation is radically different as it heavily uses OS-level threads to power its goroutine scheduling.

Go garbage collector is running in a separate thread, thus can't survive the fork. It seems like any other use of goroutines would also break.

I'm assuming that disabling the garbage collector would leak memory, although that's probably not a huge deal for some use cases.

Perhaps the workaround that I described above is the safest solution.

@tdewolff
Copy link
Owner

Yes, I can confirm this. Adding multiprocessing.set_start_method('spawn') before calling Pool() prevents hanging.

@tdewolff
Copy link
Owner

This problem is not fixed unfortunately with the recent release using CFFI...

tdewolff added a commit that referenced this issue Nov 2, 2023
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