Geany hangs on saving a certain file (reproducible) #815
Comments
|
The "filename:permission denied" is the message returned from the operating system to Geany so its probably right. What are the actual permissions of the file and the directory it is in? Is Geany running with the same permissions as the other editors? What are the values of the settings discussed in this wiki entry and does the behaviour change when you change the settings. In particular does it work when all three options are off. |
|
Are you sure the file isn't locked while trying to save it in Geany, for example by having the Ruby console or whatever uses it open? Windows is weird about clobbering files while they're open. |
I have write permission to file and directory - after all, I could change the file with a different editor.
I think it does. At least I didn't run it in "Windows Elevated Mode". Is there something else I could check in this respect?
Just the defaults. use_gio_unsafe_file_saving is on and the other two are off.
Well, I don't know the Windows locking policies so much, so let's put it like this: It's quite possible that another file was holding kind of a lock - not a "real" file lock, because there were such I lock, I could not have modified the file with another editor. However, elextr pointed me to the settings for saving the file, and from my settings, Geany seems to directly overwrite the file, while the other editor I used, likely created a temporary file first and then renamed. Maybe Windows has a locking mode, which allows the latter, but forbids the former. This theory is also supported by the fact, that the locking problem went away, after rebooting Windows. However, the real "bug" IMO is not the file locking itself (as annoying as it might be), but that Geany gets unresponsive if this occurs. Even if we really have the case a "legal" file locking, I don't think that Geany should become unoperational. |
I hope you can still reproduce the problem? So far you are the only one
Indeed, but as I said above, nobody else has (so far) reproduced the
|
|
I finally can reproduce it. Once you know it, it is simple ;-) (1) Open a sufficiently large text file (more than 25 lines) using Geany. You will get the error message about the file lock, and there is no way to remove the message and continue working with Geany, until the 'more' command is terminated. |
Do you happen to have any plugins active like the save actions one? |
Good point. I have one activated which - I'm using the German version - is called "Speicheraktionen". It came with Geany. This must be the one you are referring to. Now, if I disable it, and try to save the file, and get the error pop up, the popup goes away. This means that it is this plugin which causes the problem! |
|
The purpose of the So there is no bug. |
|
This is definitively a bug, the plugin should handle the error gracefully rather than making Geany unusable and causing the user to potentially lose data. At the very least it should throw up a dialog and allow the user to get out of the inifinite loop to save their work. Re-opening since it's a core plugin, even though the bug isn't inside Geany itself. |
codebrainz
added
plugin
saveactions
|
Agreed with @codebrainz |
|
@kugel- see the discussion I had with @codebrainz on IRC. The plugin isn't doing anything wrong, it is asked to save the file on focus out of scintilla, and going to the dialog to click "ok" is a focus out for scintilla, so it tries to save the file and gets the dialog which causes focus out ..... Suggestions welcome for how to fix it without stopping the plugin from doing its job of protecting users asses by saving as requested. |
|
I agree with @codebrainz that the plugin is in fact doing it wrong. |
|
Whats it doing wrong? |
|
Technically, the plugin is not behaving "wrong", because it exactly does as advertised. Still, what is doing is dangerous: If the file is not locked by me, but - as in case of a file on a network share, which might be locked by a completely different person, and I even don't know by whom -, I'm losing all my unsaved work. How about this: Make the popup-dialogue non-modal and modify the plugin in that way, that it does not show a popup for a certain file (to be saved), if there is one still open. Another possibility would be that the plugin - if Geany plugins have access to the status bar - do not show up the dialogue box, but instead display a message "file can't be saved because it is locked". While this is not a very user-friendly way - messages can easily be overseen -, it's still better than risking that work gets lost. A third possibility would be that the plugin, if we change the popup. Instead of having only one button ("OK"), we now have two buttons ("OK", "Save as..."), which permits the user to save the file under a different name. This would be handy even if the plugin is not active, because we always may run into the problem that our file has been locked and we can't do anything against it, but at least we want to quickly save our work. |
|
@rovf thanks for trying to help nail down the details. The only one snag with those suggestions is that the dialog is not the plugin's, its the general Geany save code that does the popup, no matter what is trying to do the save, this plugin, geany itself, or some other plugin. So adding extra buttons is just for this plugin is not possible, its going to affect much more than just this plugin. Maybe adding extra options is a good idea, just its involving more than the one plugin and so needs more careful consideration. |
|
As for the extra button, I actually thought that it makes sense in the general save dialogue, not only just for the plugin. For example, someone is locking my file. I don't know who it is or how to reach that person, but I need to save my work somehow. In a normal "save" dialogue, the pop up would occur, I would click OK, then go to "File" and "Save As..". If the popup dialogue already has a "Save As" button, it would more convenient to use. As for my other two suggestions, implementing them would mean that the plugin would not use the normal "save file"-API, but has to implement a modified version. If this is not practical, a poor-man's solution would be, that the plugin would first check, whether file and directory have write permission, and only if they do, invoke the "save file" function. This is not a completely clean solution, because between checking the permissions and actually saving the file, there is a tiny time slot, during which the file might become locked. This should be very rare though. Even then, it is easy to fix, I think: I guess the plugin is using some API function to save a file. One could extend this API by an additional parameter, which defines what to do in case of error: Show a pop-up, or return an error message to the caller (i.e. the plugin), which then handles the error in its own way. |
Yes it would, but the impact may be a significant refactoring of the save code, since ATM The last thing a plugin would want to do is to re-implement save, remember there are three options that control how its done, each of which has a different use-case so it will reduce the utility of the plugin if they are not all available. Also "save" isn't just writing the buffer to the file, there is the issue of encoding, and handling bad characters, and BOMs etc. And performing file metadata checks portably is not easy, and then they can be wrong if you are not careful, we have had issues with remote filesystem data caches giving bad data (though hopefully we have solved that now). It is not sensible to re-do that all in a plugin. Yes, it is possible to change the API, but that then affects all the other plugins that use it. Adding new API is also possible, but again that needs changes to Geany to make the new and old API work together. And lastly save is a critical function as you have pointed out, we need to be very careful making changes to it, we don't want to introduce bugs in the normal save operations, that could affect ME :). |
|
As for changing the API, the usual solution would be to add the new parameter and provide a default argument for it (and the default would be the "normal" behaviour, so other plugins don't even notice that there is something new). If I am not mistaken, Geany is implemented in C, and C doesn't have the feature of default arguments builtin, but there are several ways to achieve this functionality in C too. All the other arguments you gave, are really convincing, so IMO, the best (and maybe easiest) way would be to extend the API. |
Its code contains no form of error handling to detect, correct, or mitigate a catastrophic failure caused entirely by the plugin itself. It's basically doing the worse possible thing (nothing), which is directly responsible for causing Geany to enter into a state where all unsaved data will be lost and the user is forced to kill the Geany process or reboot. If it wasn't doing it wrong, that wouldn't happen. |
Yes, C++ overloaded functions would be a nice alternative too, but unfortunately ....
So long as one of the options of the new API matches the semantics of the old API then the old API could just call the new one with the appropriate options. There is a common idiom of having something like "save", "save_2", "save_3" etc when new interfaces are needed and the old one is to be left unchanged. The thing is to design an API and semantics that minimises the risk of introducing bugs in the normal save operations. |
|
You can even stick with only one "save" function and just simulate defaults. Say your function has the signature where X, Y and Z are some types, and you would like to simulate where setting the bool parameter to true would cause the new behaviour - no popup on failure, but storing the error message in a global variable save_last_error. To do this in C, you would use a variable argument list, i.e. and use va_arg to scan for the parameters which are present, and deduce the value of the bool from it. |
|
In general we try to make use of whatever type safety we can get the compiler to provide, especially in the plugin API which is intended to be used by programmers of varying ability. Since plugins are part of the Geany memory and process they can crash Geany. So I don't think we would want to deliberately lose type safety through the use of varargs in the plugin API in cases other than well known uses, such as printf like formatting. @b4n, any comment? |
|
I suggest looking if we can do it without API change. @codebrainz mentioned the plugin's code is crap so perhaps it can be fixed inside it. |
I didn't mean to imply the code is crap per se, just that it doesn't handle any error conditions on saving, likely due to its authors having never experienced such a scenario. But yeah, I'm quite sure it could be handled by improving the robustness of plugin's code, without modifying the API. Somewhat related, while poking around in its code, I did notice a potential security issue/race condition where the code circumvents a feature of |
@codebrainz it would be good if its possible, but I currently can't see how without breaking the plugins existing, more common, use-cases just for this very rare problem. Hence my insistence on details. But also I definitely don't want to re-factor the existing save code so an external solution is good if it is possible.
We could probably make the Geany file write code use the fd, but unfortunately neither Glibs |
|
@codebrainz: You consider it a very rare problem, but if you think again about my example of files being locked accross a Network share, this is not so rare. If I just want to protect myself from locking a file on my own disk, I can use tools like LockHunter to find the process which is doing the locking, and force the lock to go away (which at least often is the right thing to do anyway), but this would not work with Network shares. I understand your argument about compile-time type safety in your C code, but even if you followed my suggestion of going va_arg, you give up type safety only for one (i.e. the last) parameter in the signature of only one function. If you consider this too dangerous, you can still use cover-functions, which are type safe: You rename the existing 'save(X,Y,Z)' into a 'save_general(X,Y,Z,bool)', and then define two functions 'save(X,Y,Z) and 'save_nonmodal(X,Y,Z)', which just forward to save_general, but have different parameter signatures. If you do this, you stay typesafe, and existing applications just have to be recompiled. Only inside that one plugin, we have to call 'save_nonmodal' and implement an appropriate error handling. Maybe an alternative way would be to offer in the plugin configuration a checkbox, that, if selected, causes the auto-saving NOT do be done on files residing on a Network share. This would be simpler to implement (since we don't have to care about error handling), but at the cost that it might need some verbosity to explain to the user, why selecting this checkbox might make sense. |
|
@rovf I think it was me who said it was rare, not @codebrainz :) My justification is simply that we regularly get people with issues editing over network filesystems where the save actions plugin is recommended to them. But you are the only one who has reported this issue, so it seems like it simply isn't happening in the real world as often as network filesystem glitches. What I am objecting to is suggestions that make the plugin stop working when it fails, because that removes utility with flakey networks where it might succeed the next re-try. But if its stopped it won't retry. (Now speaking totally confidentially and personally, between just you and me 😉, I think people who edit over flakey network links [just to save themselves copying the file] deserve what they lose, but we seem to have accepted them, even those editing their web pages via sshfs, and been recommending save actions to ease their pain, so we shouldn't now tear the rug out from under their feet) I think we will probably use new API rather than pseudo-defaults which is an unusual technique for the project as well as having the type safety issues. These outweigh the minimal issues of having to call it Sadly finding out that files are on a network share is not portably possible that I know of, even the Glib/GIO libraries don't offer that information (that I could find). Anyway many places you have no choice but to save on network shares since that where the home directory is, and thats all you can write into. |
I think, with bugs in general, frequency of occurance is hard to guess. For example, in the majority of my past projects, I was the only one who actually reported issues with software. Most other people either ignored the problem, or switch to a different software. Especially in the case of a free text editor, by far most users who experience this kind of problem more than once, will simple turn to a different program.
It doesn't need to "stop working". If the plugin catches the error - in the way I outlined above - and displays its own error message ("Plugin can not save this time because the file is locked"), the user knows what's going on. The plugin needs to make sure that the clock counting until the next attempt of saving the file, is reset to zero when the user clicks the OK button of the pop-up message. He has then time to save the file under a different name (if he sees no possibility to remove the lock).
I don't know to what extent the network is flakey, but in many companies, it is strongly discouraged to have any file on your local drive (becaue it doesn't get up). In my case it is an exception that the majority of my files is local, and I care about my own backup, but it is not the usual way. The problem is not so much editing over the network, but editing files which are accessible to many people, and of course this can lead to race conditions, which would also exist if I would copy the file to my local disk, and write it back when I'm done. Here too, there is not much an individual programmer can do against it.
Well, we are talking about Windows here (I never experienced similar problems when working on Linux), and I think there must be a fairly reliable way to find out, whether the file is on a network: If I type 'net use' on the command line, I can see from the format of the output, whether a drive letter points to a local directory or to somewhere on the network, and if the path is not specified with a drive letter, but as an UNC path, it always can be treated as being on the network. |
Yes you're correct, so, since the local psychic is on summer holidays, all we can use is reporting frequency. As for the assumption it may be scaring away users, this is one of the rare situations where I agree with RMS (program name removed): So we tend to do stuff for us, then those who report it, and not those who don't. Now you have reported it, we are looking for a solution, but not at the cost of reducing service to the flakey network brigade who have already contributed reports in the past. Sorry to go all philosophical on your Issue, now back to normal service :)
"backed up" I assume you mean, yes, thats why simply refusing to save "remote" files is a "bad thing" ™️. "Remote" files can be connected via local gigabit connections that are more reliable than the disk drive, to connections to a remote server via coffee shop wi-fi or overloaded zero strength cell connections using sshfs, we don't know.
Well, as @codebrainz pointed out, failure to save can still be triggered on local files too, and on Linux, for example if you or one of your scripts changed the file permissions, or the disk filled up.
It would have to disable the "save on focus out" feature, because, having had the popup, a Also I'm not sure its possible to reset or cancel the timer, you have to actually get a timeout and return But someone has to come up with a good suggestion on how to re-enable the plugin, or how to otherwise break the focus-out loop long enough to save. Because the alternative of disabling the plugin and notifying the user via a popup dialog is going to be an issue for the flakey network brigade who will get it regularly. |
|
Actually, I've looked at this. I don't understand the bug (and I can't reproduce it). Given that the focus out event is used, it shouldn't be able to enter a loop. The pop up of the dialog doesn't itself cause a new (2nd) focus-out, because the widget is not focused again since the 1st focus-out. In fact, when I try this on my Linux machine with the aforementioned method to revoke write permissions, then I don't get this loop. After clicking away the dialog Geany becomes responsive as normal and only another focus-out event will re-trigger the save action. Since the dialog is modal, there is no way the widget can be become focused again (focus-in) which is, obviously, a requirement for subsequent focus-out event. For me, the theory, the code and actual behaviour all agree. Is this win32 specific, perhaps? Anyway, I thought about a possible (assuming the loop is really caused by the error popup): Since the 1st focus-out handler should be still running a flag could be set that no new save action is queued until that focus-out handler returns. That should make it impossible that the focus-out handler triggers itself. @rovf what are your exact save actions settings? I could imagine that instead of focus-out, the timeout save action is constantly triggering, for example if the timeout is low and the system takes longer than the timeout to find that it cannot write the file (again, perhaps win32 specific). IIUC g_timeout_add() preserves the interval regardless of how long the callback takes to execute (does somebody know this for sure?) |
|
@Kugel: You mention, that the event-focus plays a role in this scenario. Do you mean by this, which (visible) Window has the focus? I'm using "focus-follows-mouse" policy, which means that the window, where the mouse is hovering, gets the focus. In any case, here are my plugin settings:
|
|
@rovf are you using windows native dialogs? |
|
Well, I got into a loop on Linux by changing the permission of the current file to read-only ... once, but could not repeat it. All other times the popup showed twice only, and when oked then went away until the next timeout. @kugel- could not make it fail at all. Maybe there is a race condition somewhere or some other condition is needed. While it was hung I attached gdb which showed it was in mainloop poll with document_save up the backtrace, all as expected. But it was too hard to try to trace when the mainloop returned. |
|
What is meant by "windows native dialogs"? I have plain Windows 7, configured to "focus-follows-mouse". As with X, you can configure Windows, whether a window should get the focus by moving the cursor on top of it, or by moving the cursor in clicking on the Window. The main difference to X is, that is a bit cumbersome to change the configuration, and it doesn't always work reliable ... well, it's Windows, after all. |
|
Could it be that windows wants to focus Geany's main window but GTK wants to prevent that (because of the modality of the error dialog) but is unable to achieve that without spurious focus-in and focus-out events? |
|
@rovf, the message dialogs are always generated as windows dialogs, not as GTK dialogs. The option I was thinking of (in various) only affects the open/save/save as dialogs which can be selected to use GTK or windows dialogs. |
|
@elextr : I was not aware of this. I found in my settings the checkbox "Windows-typische Dialoge nutzen" (=use Windows dialogues), which is unchecked. I also found - this might be relevant too - the checkbox "Autofokus", which is checked. I would interpret this in the way that GTK dialogs are used, and that child windows automatically get the focus. Should I change one of these settings and see, whether the error still occurs? |
|
@rovf since the dialogs we are looking at are always made as windows dialogs on windows and GTK dialogs elsewhere, there is no need to try the "use windows dialogs" since it doesn't apply to the message dialog from save, but thanks for the offer. It might be worth turning off the autofocus, though I expect clicking "ok" will change the focus anyway. |
|
@rovf . Is this issue still valid today? This PR is over 8 years old now. If it is not, can it be closed? Thanks. |
|
I'm now on Geany 1.36 and haven't seen the issue again. In the course of the update, I also upgraded to Windows 10, so I don't know whether the problem was solved in Geany or some dependency on the operating system. |
|
AFAIK nothing was changed in Geany that is relevant between 2015 and 2019 (when 1.36 was released) and the save actions plugin had no code changes in that period? So who knows, maybe windows 10 changed something, or you have just been lucky. But will close anyway. |
I switched today from Geany 1.25 to 1.26. With the new version (running on Windows 7 64bit), I have (reproducibly) a strange problem with one file (its name is
.irbrc) which I had created with 1.25: When I want to save the file, an error popup occurs (see screenshot) which gives a "permission denied" message. Moreover, this popup can not be clicked away (more precisely, when I click on OK, the popup seems to disappear for a fraction of a second, only to reappear again). I think, this behaviour of the popup window is clearly an error, because it is a modal dialogue, and I have to kill Geany with the task manager in order to continue my work. It could be that Geany, after I clicked OK, tries again to fulfil the Save request, and finds - for whatever reason - the file still write-protected and brings up the same dialogue again.BTW, I don't think that this is really a permission issue, even though the message says so, because I can happily edit this file with other text editors. I don't know why this file is special. It is neither the name nor the content - I can use "save as" to save the file into a different directory.
The text was updated successfully, but these errors were encountered: