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

Cancel indexing job when closing filepropsdialog #20

Merged
merged 1 commit into from
Mar 21, 2016

Conversation

npmiller
Copy link
Contributor

Fixes: lxqt/pcmanfm-qt#328

fm_job_cancel is enough clean up because it will trigger the finished signal and the usual job cleanup will then take place.

@tsujan
Copy link
Member

tsujan commented Mar 20, 2016

Please change it to this:

  if(deepCountJob) {
    g_signal_handlers_disconnect_by_func(deepCountJob, (gpointer)G_CALLBACK(onDeepCountJobFinished), this);
    fm_job_cancel(FM_JOB(deepCountJob)); // emits finished signal
    g_object_unref(deepCountJob);
  }

lxqt/pcmanfm-qt#328 (comment) may be fixed in this way

And would you please test the commit afterward? Here, I can't reproduce the crash, perhaps, because my laptop is too fast.

@tsujan
Copy link
Member

tsujan commented Mar 20, 2016

g_object_unref(deepCountJob); my not be needed.

@Ilya87
Copy link
Contributor

Ilya87 commented Mar 20, 2016

How to reproduce: open your terminal app, killall pcmanfm-qt, gdb pcmanfm-qt, run, properties on your home folder or other folder with many files, press Esc button as soon as possible after clicking on properties.

@npmiller
Copy link
Contributor Author

Ok, I've updated the fix, it should be good now, there's several things going on:

  • Stop the timer before cancelling the job, should be fine the other way around since the timer callback checks if the job is cancelled, but I think it's safer this way.
  • @tsujan suggestion and handling the cleanup in the destructor instead of using the finished callback
  • Only delete the ui at the last step of the destructor

Both the two last point seem to fix the issue, this is because the finished callback accesses the filesprops dialog window which seems to be in a bad state after the ui has been deleted (not exactly surprising). So either deleting the ui after cancelling the job, or not accessing the window after cancelling the job seem to work. Doing both here just to be on the safe side.

@tsujan
Copy link
Member

tsujan commented Mar 20, 2016

OK, pressing Esc was the solution (as soon as I wanted to press a dialog button, indexing was finished). I saw the crash but after #20 (comment), I can't see it anymore. @npmiller's new commit should fix it too. Please test it!

@Ilya87
Copy link
Contributor

Ilya87 commented Mar 20, 2016

No crash!

@PCMan PCMan merged commit b2f9acf into lxqt:master Mar 21, 2016
@agaida agaida added this to Done in Pull Requests Apr 1, 2018
@agaida agaida added this to old in PRs Nov 5, 2018
@agaida agaida added this to Archive in PRs Nov 5, 2018
@agaida agaida added this to Old Archive in Pull Requests Mar 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
PRs
  
Archive
Pull Requests
  
Old Archive (fucked up)
Development

Successfully merging this pull request may close these issues.

None yet

4 participants