This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Product Version = Sun ONE Studio 5, Standard Edition (Build 030406) IDE Versioning = IDE/1 spec=3.42.1 impl=030406 Operating System = Linux version 2.4.18-27.8.0 running on i386 Java; VM; Vendor = 1.4.2-beta; Java HotSpot(TM) Client VM 1.4.2-beta-b19; Sun Microsystems Inc. Java Home = /usr/local/java/j2sdk1.4.2/jre System Locale; Encod. = en_US (f4j_ee); UTF-8 Home Dir; Current Dir = /usr/local/home/delphym; /opt/s1studio5_se/bin IDE Install; User Dir = /opt/s1studio5_se; /home/dm103276/studio5se_user ------------------------------------------------------------------------------- I discovered this ugly behaviour imediately I received patch for issue #32760 (have a look there for more info, please) which is stopper for Nevada (ClearCase users) The problem is when users share somehow (ClearCase users) their files and happens that the one file is edited at the same time by more then one user! I recomanding, it is common scenarion for CC users!!!! Easy Reproduction on a local FS: ================================= 1.Open a file in editor and start to edit. Don't save it !!! 2.Open this file in external editor and make some changes in this file. Save it !!! 3.Now, WITHOUT FOCCUSSING the Source editor in the ide, click to save icon on the toolbar of main window, or use a File->Save menu (If you foccus the editor, The editor itself is nice and take care about it) The file is then stored on the disk in the shape/content as it was left in the Source ide's editor! Which is very ugly for user/ejb(property manipulation)/CC user who were editing the same file externaly (from ide's editor) Scenario for G-CVS where with CC profile is very common situation: ================================================================== The patch from issue #32760 is fixing the synchronization of proper statuses beatween Explorer and the Editor only ....
Dan, I've tried it and IDE asks me to reload modified file! What's wrong ?
Dan, I cannot roproduce it too. Can you realy reproduce it ?
Assigned to Peter, for investigation.
It's really unbelieveble, but it's reproducible :(. IMPORTANT: don't focus editor before you push Save!!! IMHO: It isn't regression, it works this way from the beggining of the NB..
Excuse me, I didn't ommited some details for reproduction... But Marian, now knows how to reproduce. Marian told me that he's using MDI mode, but I prefere the tradional one SDI. And that way didn't hurt us for long time, but only show that there is a bug. Problems occure when you apply MArtin's patch from issue #32760 (I think you don't need it, but rather for sureness:) ================================================================ 1) Mount G-CVS FS and open any file in editor. (I chose [up-to-date] 2) modify this file and DON'T SAVE IT!!!! 3) open the file in external editor and modify it and SAVE IT NOW! And from NOW, AVOID to GET FOCCUS TO Source Editor 4) In the Explorer on the file or on the dir perform CVS-Refresh now see, that file doesn't contain '*' (asterick) sign in the editor's title. 5) Open this file again in the external editor and see, that the changes made in this external editor are away!!! It is because of tat the CVS->Refresh updating file statuses, and looking for last file modification, and than it calls Save action, which doesn't seem to care about if the file was modified externaly. This sitation is critical for eg. modifiing properties of EJB when you don't have even opened editor! It is critical for ClearCase users too... (CC'ing some people)
It seems to be completely other kind of problem I got first info about :-). Question: what kind of FS is G-CVS? The problem seems to be in 3). You don't get the reload dialog after you saved the file externally, don't you? This seems to be the problem and not 4).
If the above assumption is right, there needs to be investigated why in CloneableEditorSupport wasn't callecd checkReload (somewhere in Env) method, after the file was saved externally.
> why in CloneableEditorSupport wasn't called checkReload() That's the point. Editor is not involved here at all. It's just a way how to easily make a modified file. As soon as the file gets modified the editor is not touched. It's not a mistake of CloneableEditorSupport! CloneableEditorSupport calls checkReload() as soon as it gets focus, but in this case it does not get the focus. It's not important where the file got modified, important is that the saving process does not check whether the file was modified externally in the mean time and overwrites the file.
> Question: what kind of FS is G-CVS? Generic (command-line) CVS. Mount: Version Control -> Generic VCS -> select CVS profile.
I'm evaluating it from editor point if view. PetrZ gave me a tip to try.
One think is changed in 3.5 release, we haven't auto refresh on filesystem, it means nobody asks for refresh on files, so IDE doesn't know about file modification until you move focus to editor. In [nb3.5](200304082350), [jdk1.4.2](b19) after pushing "Refresh folder" from the menu, file isn't refreshed! - again none notification about externally file modification.
Created attachment 9838 [details] binary patch
Created attachment 9839 [details] source code patch
I attached binary patch (put the file into your lib\patches folder) and source code patch. PetrZ, could you please look at it and review it? Thanx. I must say that I'm not familiar with this source code and so I'm not 100% that this is the right way to fix it and that it will not break anything. On the other hand the fix seems to be trivial. It is just additional check and so it most probably should not break anything. Comments welcome. Needs to be properly tested. As for your last comment Marian about refreshing folder without any effect, that seems to me as as a separate issue. RadekM will know more. Looks like some optimization? I do not know.
So there seems to be two problems. 1) To the fix: when user will save it, and says no to reload, the doc won't be saved. 2) Also it doesn't fix CloneableEditorSupport, (e.g. properties will be still wrong) But for both cases there doesn't seem to be a nice solution: 1) there is needed to invoke refresh and the associate the upcoming reload dialog with that refresh, more logic would be needed to introduce. 2) It is missing facility of refreshing of environment in the API design (it would be needed in the CES.Env class, btw. it is necessary to mention that there is already almost same problem with missing isReadOnly ), i.e. it would require API enhancement, to be able fix it cleanly. It seems that the original design was insufficient.
Yarda, as expert on editor package I would like to hear your opinion on this as well. This issue is really ugly.
Created attachment 9848 [details] another possible patch (better than previous one)
I attached another patch. It fixes issue #2 identified by PetrZ by using CloneableEditorSupport.Env.getTime() and by reimplementing this method in all places in NetBeans (actually just 4 places what is good for us). This would probably have to be done in all Nevada sources as well. All implementors of CES.Env should reimplement the getTime() method, otherwise there is a risk that modifications in their editor will be lost. On the other hand this issue itself requires a several coincidences to happen at once to be reproducible so if we fix most of the common editors the chance of this bug is again much much lower. The issue #1 is not solved but I consider it P5. PetrZ, Yarda, what do you think about it? Do you see any problems? Any possible improvements? I'm not saying it is nice solution, but I do not see any other. And the change itself is quite trivial and should not have any sideeffects nor performance impacts. Thanx for your review and help.
David, please provide binary patch fo testing, if it's final.
I will, but first I would like to hear comments from Peter and Yarda.
Yarda in offline talk had good comment: the data you lost are data which are still available in VCS. You can just ask VCS to checkout file again, right? So is this really P1 data lost? If the file was modified externally not by VCS but by a user then this problem can still happen, but on the other hand user is modifying the same file in two editors....she is asking for troubles and gets what she deserves. :-)
David wanted me to provide a review: 1. I think that the solution that compares the file modification time and last save time is the right one. 2. I do not like the "return" statement, in case there is a problem. I suggest different solution. The text package is special as sometimes there is a need for a communication between the code and the user during save, load calls that cannot without risk of deadlock just open AWT dialog and ask. For this purpose org.openide.util.UserQuestionException has been introduced and is being used in loadDocument. It is not nicest solution, but it is a working solution. I suggest to use it in saveDocument as well: Instead of just returning throw UserQuestionException asking that the file on disk "Is modified. Save or not?" and in the code that calls the save method (probably SaveAction and SaveAllAction) and that can use AWT, catch the exception, show question dialog and either repeat or give up the action. After this change the expected behaviour from user point of view will achieved and that is the goal is it not?
> , but on the other hand user is modifying the same file in two > editors....she is asking for troubles and gets what > she deserves. :-) Yes, you're right :-) But that's the way how ClearCase Dynamic view works :-(((
Yarda, your suggestion will not work, because reload task is asynchronous. I have working patch which I will attach tomorrow. I discovered two additional problems: The first one is P3 issue 26407 which I had to reopen - if caret is at the end of file and reloaded file is shorter the exception is thrown. The exception is harmless, the caret will be moved to the end. I already have fix and will commit it tomorrow. The second is P2 issue 32846 which I discovered during testing of patch for this issue. It is independent on this one and must be evaluated. It causes similar problem as described in this issue, but is limited to some types of files only and reproducible scenario is even more obscure. This issue depends on the 32846, but fact that 32846 exist for some time and have never been noticed sounds to me that it is not that important to block this one. Final word has of course QA.
Created attachment 9917 [details] patch solving the issue
Created attachment 9918 [details] openide binary patch
Created attachment 9919 [details] properties module binary patch
Who's gonna test this ? Mariane ?
Attached is source code patch for the review and two binary patches. The openide one must be put into lib\patches folder; the properties module patch must be put into modules\patches\org-netbeans-modules-properties folder. Please test and review. The fix calls CES.Env.getTime() during the documentSave(). The getTime method was reimplemented to call FileObject.refresh(). I grepped all the NetBeans source code and also Nevada sources and only place where I found implementation of this environment was Properties module and DataEditorSupport. Both places were fixed. The fix should not have any sideeffects, but should be properly tested. It also changes (undefined) behaviour of two methods: * CES.Env.getTime() - it has to return uptodate time of the input stream * CES.saveDocument() - it can return without throwing an exception and document still can remain modified. I do not know whether I should document it in JavaDoc, because current JavaDoc does not mention nothing about how these two methods should behave. During the testing of reload of properties files you can bump into problem reported as issue 32846.
Jirka, please try Dan's steps to reproduce, I will test it only with LocalFS. Can you help us?
I am pretty busy now with metrics however okay I will try to reproduce it.
So, my judgement is that the fix is okay. I checked both test cases with Generic VCS filesystem and an external version of a file was preserved if user answered "Yes".
patches verified, thanks to everybody for cooperation.
The fix solves this problem. Well, it changes the API slightly (saveDocument finishes but doc may not be saved - what should be pointed out in javadoc). On the other hand, there doesn't seem to be a nice way fixing this problem and this is probably the best possible hot fix for this time. It seems that the API definition prevents nicer solution. So it is up to decision whether it is worth to apply this fix. I personally think that yes.
Thanx Peter. I agree with you. I'm going to post request on reviewers@ so that other people can comment it.
I posted review request, filed INF and now I'm waiting 24hours. If there are no comments from reviewers I will commit it into trunk and mark this issue as fixed for 4.0.
approved for 3.5
Fixed both in trunk and nb35: Checking in properties/src/org/netbeans/modules/properties/PropertiesEditorSupport.java; new revision: 1.67; previous revision: 1.66 Checking in openide/loaders/src/org/openide/text/DataEditorSupport.java; new revision: 1.4; previous revision: 1.3 Checking in openide/src/org/openide/text/CloneableEditorSupport.java; new revision: 1.85; previous revision: 1.84 Checking in properties/src/org/netbeans/modules/properties/PropertiesEditorSupport.java; new revision: 1.65.2.1; previous revision: 1.65 Checking in openide/src/org/openide/text/CloneableEditorSupport.java; new revision: 1.81.2.2; previous revision: 1.81.2.1 Checking in openide/src/org/openide/text/DataEditorSupport.java; new revision: 1.25.2.1; previous revision: 1.25
verified in [s1s](030417)