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

Deadlock when exiftool process is killed before parent #68

Open
agorman opened this issue May 27, 2023 · 0 comments
Open

Deadlock when exiftool process is killed before parent #68

agorman opened this issue May 27, 2023 · 0 comments

Comments

@agorman
Copy link
Contributor

agorman commented May 27, 2023

There is a deadlock in the code if the exiftool process is killed before the parent process calls Close(). The deadlock occurs here https://github.com/barasher/go-exiftool/blob/master/exiftool.go#L183. It happens because r and w created in NewExiftool are never closed.

This can happen pretty often when the parent program is killed using with CTRL+c. By default on Linux exiftool is run in the same process group which causes the signal to reach the exiftool process before the parent has the time to close it cleanly.

One possible solution would be to run exiftool in it's own process group so signals to the parent aren't passed to the child. The problem with this is that the way process groups are handled is OS dependent and code would need to be added to handle different operating systems. Another problem is that it's desirable to make sure exiftool is closed even when people aren't properly calling Close in their code.

Anther possible solution would be to add a goroutine in the NewExiftool after cmd.Start() that can close r and w when the process exits.

go func() {
	e.cmd.Wait()
	r.Close()
	w.Close()
}()

Here is a minimal failing case https://gist.github.com/agorman/6ef72d4be151326e39de980247c6ea46. To reproduce it run it against a directory with a bunch of images and do a CTRL+c or kill the exiftool process.

I'm happy to write a pull request but wanted to get your input on possible solutions before doing that.

Thanks!

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

1 participant