#14889 closed defect (fixed)

wxFileSystemWatcher: better processing of inotify IN_MOVE events

Reported by: dghart Owned by:
Priority: low Milestone: 3.0.0
Component: base Version: stable-latest
Keywords: wxFileSystemWatcher IN_MOVE Cc:
Blocked By: Blocking:
Patch: yes

Description

IN_MOVE inotify events are produced both by what a user sees as moves and by renames.

  • A rename gives an IN_MOVE_FROM event immediately followed by an IN_MOVE_TO. Both have the same path.
  • A move from a watched dir will result in only an IN_MOVE_FROM. A move into one gives only an IN_MOVE_TO.
  • A move within a tree of watches gets both IN_MOVE_FROM and IN_MOVE_TO. These aren't currently processed optimally.

The problem is the paths. In this situation the filename is the same for both events but the path differs. If the events arrive far enough apart that they aren't recognised as a pair, they will be dealt with separately in ProcessRenames() and the paths of both resulting wxFileSystemWatcherEvents will be correct. However for me they arrive in the same batch of events, and so are treated as a pair. That means the path is extracted from only the IN_MOVE_TO event's watch, so the owner doesn't receive a valid 'from' path.

The patch fixes this by passing paths taken from both of the original watches. This won't affect a true rename as both paths will be identical, or unpaired IN_MOVEs as ProcessRenames() is unchanged. I've also added internal documentation for IN_MOVE processing.

Attachments (1)

fswatcher_inotify.diff download (3.4 KB) - added by dghart 23 months ago.

Download all attachments as: .zip

Change History (4)

comment:1 follow-up: Changed 23 months ago by vadz

  • Milestone changed from 2.9.5 to 3.0
  • Priority changed from normal to low

I'm not sure if I agree with the added comments. "Move" and "rename" are the same thing, so it hardly makes sense to explain why are they treated in the same way...

I do agree with the changes themselves, of course (although I wonder what happens if the file is moved from outside the watched directory and immediately moved outside it again, does the code handle this case correctly?), but would prefer to modify or simply omit the (slightly misleading IMHO) comments. What do you think?

Changed 23 months ago by dghart

comment:2 in reply to: ↑ 1 Changed 23 months ago by dghart

Replying to vadz:

I'm not sure if I agree with the added comments. "Move" and "rename" are the same thing, so it hardly makes sense to explain why are they treated in the same way...

Yes, a unix programmer will presumably know this; but it's perhaps less obvious in an inotify context; especially in view of the current code, which refers only to rename.

I do agree with the changes themselves, of course (although I wonder what happens if the file is moved from outside the watched directory and immediately moved outside it again, does the code handle this case correctly?)

I've not experienced that situation (yet) but I think it does. ProcessNativeEvent() doesn't check the type (TO or FROM) of an IN_MOVE; it just puts the watch path of the first to arrive in wxFileSystemWatcherEvent's 'path' field and that of the second in newPath (before the patch these were identical). So matched events will provide useful information to user-code, as would the separate events if unpaired.

but would prefer to modify or simply omit the (slightly misleading IMHO) comments.

I've toned down the comments, but I'm sure that the description of how the code handles the three different situations will help anyone touching this code in the future.

Nevertheless the fix is far more important, and I won't be offended if you remove the comments entirely.

comment:3 Changed 23 months ago by VZ

  • Resolution set to fixed
  • Status changed from new to closed

(In [73162]) Handle IN_MOVE inotify events better.

Set the new path correctly for moved or renamed files.

Closes #14889.

Note: See TracTickets for help on using tickets.