search

michaelrsweet / pappl

PAPPL - Printer Application Framework
https://www.msweet.org/pappl
Apache License 2.0
307 stars 49 forks source link

deadlock happens when delete printer #297

Closed tangyanli closed 11 months ago

tangyanli commented 11 months ago

Hi, Michael:

I reported the bug on https://github.com/michaelrsweet/pappl/issues/141, and there isn't any reply. So I report this new one.

Describe the bug The pappl (v1.40) deadlocks when delete a printer on web page.

To Reproduce Steps to reproduce the behavior:

  1. Build and run legacy-printer-app
  2. Add a legacy printer on the localhost:8000 (eg. Canon driver)
  3. Delete the printer on the web page.
  4. The pappl (web-page) deadlocks.

Expected behavior The delete operation should success.

Screenshots I tried several printer-apps, the hp-printer-app cannot reproduce this issue. The ps-printer-app can reproduce depends on the selected driver.

System Information:

Additional context I analyses the code, when the deadlock happens, the call stack as below:

papplPrinterDelete                                                              
    _papplRWLockWrite(system)       ★(First Time)                                               
    cupsArrayRemove                                                         
        _papplPrinterDelete                                                                         
            _prDriverDelete                                                 
                papplSystemRemoveResource                                               
                    _papplRWLockWrite(system)       ★(Second Time)                                          
                        cupsArrayRemove(system->resorce)                                    
                    _papplRWUnlock(system)                                          
    _papplRWUnlock(system)

The _papplRWLockWrite() is invoked twice before unlocking. So I tried to comment the papplSystemRemoveResource() in function _prDriverDelete(), then this issue cannot be reproduced.

Then I added the "-DDEBUG", and when deleting the printer, the output is as below:

0x7f83d2c41a80/_papplClientCreate: wrlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d2c41a80/_papplClientCreate: unlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d2c41a80/papplSystemRun: wrlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d2c41a80/papplSystemRun: unlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d2c41a80/papplSystemRun: rdlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d2c41a80/papplSystemRun: unlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d2c41a80/papplSystemRun: rdlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d2c41a80/papplSystemRun: unlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d2c41a80/papplSystemRun: rdlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d2c41a80/papplSystemRun: unlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d2c41a80/papplSystemRun: rdlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d2c41a80/papplSystemRun: unlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d1c39640/papplSystemGetHostName: rdlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d1c39640/papplSystemGetHostName: unlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d1c39640/_papplSystemFindResourceForPath: rdlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d1c39640/_papplSystemFindResourceForPath: unlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d1c39640/papplSystemAddEvent: rdlock 0x7f83c8000bb0(legacy_1)
0x7f83d1c39640/_papplSystemAddEventNoLockv: rdlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d1c39640/_papplSystemAddEventNoLockv: unlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d1c39640/papplSystemAddEvent: unlock 0x7f83c8000bb0(legacy_1)
0x7f83d1c39640/papplPrinterDelete: wrlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d1c39640/_papplPrinterDelete: wrlock 0x7f83c8000bb0(legacy_1)
0x7f83d1c39640/_papplPrinterDelete: unlock 0x7f83c8000bb0(legacy_1)
0x7f83d1c39640/papplSystemRemoveResource: wrlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d1c39640/papplSystemRemoveResource: unlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d1c39640/papplPrinterDelete: unlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d1c39640/_papplClientDelete: wrlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d1c39640/_papplClientDelete: unlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)
0x7f83d2c41a80/_papplClientCreate: wrlock 0x559d7aa56a60(CUPS Driver Retro-Fit Printer Application)

I don't know how to investigate further, please help.

michaelrsweet commented 11 months ago

@tangyanli In general, when you encounter a new issue, it is best to report it in a new issue and not as a comment to a bug that was closed two years ago...

I will investigate further...

michaelrsweet commented 11 months ago

OK, looking at the PAPPL code and the backtrace, the issue is that driver delete callback in the printer application you are using is (incorrectly and unnecessarily) calling papplSystemRemoveResource.

It seems that I might want to update papplPrinterDelete so that the printer can first be removed from the system object and then delete the rest of the printer object without holding the system rwlock...

michaelrsweet commented 11 months ago

[master 249316b] Fix potential deadlock caused by driver deletion callbacks (Issue #297)

[v1.4.x 745e43c] Fix potential deadlock caused by driver deletion callbacks (Issue #297)

tangyanli commented 11 months ago

Thanks, Michael! I have tested it, the issue is fixed. And I now can delete the printer smoothly.

I still have a query.

OK, looking at the PAPPL code and the backtrace, the issue is that driver delete callback in the printer application you are using is (incorrectly and unnecessarily) calling papplSystemRemoveResource.

For the legacy-printer-app, which is the best way to delete the printer's related resource (e.g the localization strings) allocated by the function papplSystemAddStringsData().

michaelrsweet commented 11 months ago

@tangyanli WRT printer resources, as long as the resource path is relative to the printer's prefix ("/ipp/print/PRINTERNAME/RESOURCENAME.EXT") then the delete callback doesn't need to call papplSystemRemoveResource. The change I made will allow the delete callback to remove resources, however, just in case the resource isn't under the printer's path.