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.

Sign up for GitHub

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

Jump to bottom

mc hangs accessing files within a .tar.gz within a cpio within an rpm for first 60 seconds #4147

Closed
mc-butler opened this issue Nov 19, 2020 · 10 comments
Closed

mc hangs accessing files within a .tar.gz within a cpio within an rpm for first 60 seconds #4147

mc-butler opened this issue Nov 19, 2020 · 10 comments
Assignees
@aborodin
Labels
area: vfs Virtual File System support prio: medium Has the potential to affect progress ver: 4.8.25 Reproducible in version 4.8.25
Milestone
4.8.26

Comments

@mc-butler
Copy link

Important

This issue was migrated from Trac:

Origin https://midnight-commander.org/ticket/4147
Reporter nvwarr (nvwarr@….com)

In lib/vfs/gc.c in the function vfs_expire, curr_time and exp_time are declared guint64. curr_time is initialised with a timestamp and exp_time with this timestamp minus 60 seconds. Later there is if (stamping->time <= exp_time). Prior to commit [a94dd7d] curr_time was initialised with a value larger than 60 seconds, so everything was fine. This commit changed the initialisation to a timer starting when mc is started. So for the first 60 seconds, the result of the subtraction is negative, but it is a guint64, so we just get a VERY large unsigned value and the if (stamping->time <= exp_time) is always true. So mc thinks the vfs hasn't been used recently and goes into an infinite loop.

If one opens a .rpm file with mc and goes into the CONTENTS.cpio and then tries to go into the .tar.gz there (this is the usual structure of a .rpm) after waiting 60 seconds, everything is fine. However, before 60 seconds, mc hangs. There may be other ways to trigger this, but this is how I discovered it.

The fix is to remove the subtraction of 60 seconds when initialising exp_time and add them to the left-hand side of the comparison.

Note

Original attachments:
@mc-butler
Copy link
Author

Changed by nvwarr (nvwarr@….com) on Nov 19, 2020 at 10:07 UTC

Patch to fix the bug

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Nov 19, 2020 at 12:11 UTC (comment 1)

  • Component changed from mc-core to mc-vfs
  • Version changed from master to 4.8.25

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Nov 21, 2020 at 12:52 UTC (comment 2)

  • Owner set to andrew_b
  • Status changed from new to accepted

Remembering discussion #3859, I think it's time to drop mc_timer and use g_get_real_time(). There is no difference what time to use: relative to mc start moment or relative to UNIX epoch.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Nov 22, 2020 at 12:45 UTC (comment 3)

  • Milestone changed from Future Releases to 4.8.26
  • Branch state changed from no branch to on review

Branch: 4147_vfs_timeout
Initial [e612ea50a12c70dd1d636a3086c6889db52f9ce0]

Please test.

@mc-butler
Copy link
Author

Changed by nvwarr (nvwarr@….com) on Nov 22, 2020 at 14:45 UTC (comment 4)

Branch 4147_vfs_timeout works for me. So that is another way to solve the problem. I still think that you should consider avoiding the subtraction of vfs_timeout seconds from an unsigned type, though. That's always a bit dangerous. I would consider that the real bug, which was exposed by using mc_timer and could potentially reoccur if a future change goes back to a relative time since startup of mc. That said, your fix does avoid the issue.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Nov 22, 2020 at 15:04 UTC (comment 4.5)

Replying to nvwarr:

I still think that you should consider avoiding the subtraction of vfs_timeout seconds from an unsigned type, though.

guint64 is replaced with gint64 for all related variables and structure members.

That's always a bit dangerous. I would consider that the real bug, which was exposed by using mc_timer and could potentially reoccur if a future change goes back to a relative time since startup of mc.

mc_timer is removed in the 2nd commit in this branch.

@mc-butler
Copy link
Author

Changed by nvwarr (nvwarr@….com) on Nov 22, 2020 at 15:08 UTC (comment 6)

Oops, I missed the change to gint64. That's perfect then.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Dec 12, 2020 at 17:38 UTC (comment 7)

  • Branch state changed from on review to approved
  • Votes set to nvwarr andrew_b

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Dec 12, 2020 at 17:41 UTC (comment 8)

  • Resolution set to fixed
  • Status changed from accepted to testing
  • Branch state changed from approved to merged
  • Votes changed from nvwarr andrew_b to committed-master

Merged to master: [446a031].

git log --pretty=oneline f0a3794b0..446a03135

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Dec 12, 2020 at 17:42 UTC (comment 9)

  • Status changed from testing to closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aborodin aborodin

Labels
area: vfs Virtual File System support prio: medium Has the potential to affect progress ver: 4.8.25 Reproducible in version 4.8.25
Milestone
4.8.26
Development

No branches or pull requests
2 participants
@aborodin @mc-butler