PrintHandler crash & a related fix that should not work (?)

[Peter]

I've encountered this problem, when the CefClient example on Linux crashes if I cancel the Print dialog:

Code: Select all

      case GTK_RESPONSE_CANCEL: {
        dialog_callback_->Cancel();
        dialog_callback_ = NULL;        // SEGFAULT!
        return;
      }


Then I found this issue #2422, that fixes pretty much the same problem, only with `job_callback_->Continue()` (Does this mean that the exact same bug with `dialog_callback_` has not been fixed yet?).
Here's how it looks like:

Code: Select all

  void OnJobCompleted(GtkPrintJob* print_job, GError* error) {
    job_callback_->Continue();
    job_callback_ = NULL;
  }

is changed to

Code: Select all

  void OnJobCompleted(GtkPrintJob* print_job, GError* error) {
    // Continue() will result in a call to ClientPrintHandlerGtk::OnPrintReset
    // which deletes |this|. Execute it asnychronously so the call stack has a
    // chance to unwind.
    CefPostTask(TID_UI, base::Bind(&CefPrintJobCallback::Continue,
                                   job_callback_.get()));
    job_callback_ = nullptr;
  }

Now, probably because of some other changes that my (older) CEF version does not have, this `CefPostTask(...)` didn't want to compile for me here, so I tried to understand what's going on here and fix my problem in a differentl way, but... I don't get it - how come this fix even works?

Since both `job_callback_` and `dialog_callback_` are CefRefPtr<>s, after they are assigned to NULL (and HasOneRef() == true), the object they're referring to is destroyed - so, the `job_callback_.get()` that was passed to CefPostTask should no longer be valid?

Another thing - why do we need to set `job_callback_ = nullptr;` anyway - if we know that after `job_callback_->Continue();` the whole object will be destroyed?

[upd] Oh, and also - can there be any circumstances where `job_callback_`, or `dialog_callback_` in my case would also be referenced elsewhere?
If not, can we just call Continue() or Cancel() here and return?

[magreenblatt]

What Linux distro and CEF version are you using? Does the problem reproduce with supported versions?

[Peter]

What Linux distro and CEF version are you using? Does the problem reproduce with supported versions?

Ubuntu 19.10
Unfortunately I'm stuck with this old version 3112, so I have little reason to check the newest one.

Still, I'm wondering if at least my understanding of the CefRefPtr-related logic is correct...

[magreenblatt]

If it reproduces with a supported version then you can file a bug. Otherwise, it might already be fixed.

[Peter]

I've been able to crash this prebuilt version cef_binary_84.3.3+gffc2f0e+chromium-84.0.4147.89_linux64 in the same way.
All you need to do is { right click - Print, then cancel the Print dialog } enough times - sometimes it works fine, but eventually ~PrintHandler() is called earlier than it should be.

I just added simple logging, and here's my output:

Code: Select all

 before dialog_callback_->Cancel(), HasOneRef()=1
 before dialog_callback_ = nullptr
 after  dialog_callback_ = nullptr // worked fine this time
~PrintHandler()
 before dialog_callback_->Cancel(), HasOneRef()=1
 before dialog_callback_ = nullptr
 after  dialog_callback_ = nullptr // worked fine this time
~PrintHandler()
 before dialog_callback_->Cancel(), HasOneRef()=1
~PrintHandler()
 before dialog_callback_ = nullptr
Segmentation fault (core dumped)

[magreenblatt]

Please add a bug.