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

[1.28.1] cups-browsed gets SIGKILL during service restart #298

Closed
zdohnal opened this issue Aug 27, 2020 · 5 comments
Closed

[1.28.1] cups-browsed gets SIGKILL during service restart #298

zdohnal opened this issue Aug 27, 2020 · 5 comments

Comments

@zdohnal
Copy link
Member

zdohnal commented Aug 27, 2020

Hi,

we have a CI test which uses cups-browsed - it is compiled to don't keep generated queues and use RemoteName for created local queues pointing to remote cups queues.
Then I set BrowsePoll <cups-1.4.2 server> (I used BrowseFilter to simplify things too), wait for queues to show up and then stop the service. Stopping the service takes longer than with 1.28.0 and the service is killed (log from systemctl):

Debug log
for loading one queue and
debug log when stopping the service (it takes 1m 30s to stop the service - the SIGTERM signal times out and SIGKILL is sent) are attached.

IMO the issue is the signal is ignored and one commit in 1.28.1 moved signal handlers, so I'll try if reverting that commit will help.

@zdohnal
Copy link
Member Author

zdohnal commented Aug 27, 2020

My guess is correct, reverting the commit which moved signal handling into get_printer_attributes5() fixes the issue.

@zdohnal
Copy link
Member Author

zdohnal commented Aug 27, 2020

@tillkamppeter signal handler with cancel_job() masks general sigterm_handler() so cups-browsed never shutdown itself if signal handler is in get_printer_attributes5() (probably because it is loaded in cups-browsed binary).
If the signal handler is within get_printer_attributes4(), then it is called only from driverless/driverless-fax executables, so cups-browsed is not affected.

@tillkamppeter
Copy link
Member

Fixed in commit 23a9dc4 on master. Thank you very much for your report.
The signal handlers got in with the student work for IPP Fax Out support but they are unneeded. converting DNS-SD-based URIs into standard IPP URIs and polling printer attributes are rather quick operations and there are also no problems with unclean shutdown as one had with a print job. So signal handling is overkill here and therefore I removed it.

@zdohnal
Copy link
Member Author

zdohnal commented Aug 28, 2020

@tillkamppeter Thank you for the fix! Isn't the issue serious enough to release 1.28.2? If someone installs 1.28.1 without the patch, then shutdown will be longer for that 1m 30s till SIGKILL kills cups-browsed.

@tillkamppeter
Copy link
Member

I will catch some more bug fixes and in a few days I will release.

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