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.
Summary: | Need file read-only/writeable status change event | ||
---|---|---|---|
Product: | java | Reporter: | Peter Liu <petertliu> |
Component: | Unsupported | Assignee: | Svata Dedic <sdedic> |
Status: | VERIFIED FIXED | ||
Severity: | blocker | CC: | dmladek, jchalupa, jrojcek, mentlicher, pkeegan, pnejedly, sdedic, ttran |
Priority: | P3 | ||
Version: | -S1S- | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | DEFECT | Exception Reporter: | |
Bug Depends on: | 23407 | ||
Bug Blocks: | |||
Attachments: |
Proposed patch
Binary patch for testing FTD +E. stackTraces another FullThreadDump Full Thread Dump on 1 CPU kernel |
Description
Peter Liu
2003-03-11 20:38:07 UTC
*** Issue 31915 has been marked as a duplicate of this issue. *** bug can be in vcs, java or openide, added sdedic to CC CVSGENERIC does not fire an FileEvent when the CVS Edit is done, however it would require detection of change of attributes which is not an easy task. The File.lastModified() is not affected by the metadata change, so it would require holding the actual state in reference and testing it with current state after the VCS Edit is done, it would be much worse for folder, where the recursive refresh of folder is needed. The behaviour is the same for external change of filemetadata on local fs. The workaround is simple, reselect the node representing the JavaFile. Not a P2 issue, decreasing priority to P4. How is reselecting the node going to help? I tried it but the property sheet for the node remained read-only. I tried this in the dev build and it works fine. Both editor and property sheet on the Java file. Which NB are you using? However the workaround is a temporary, there is a need to solve this problem, but it will be quite hard and it should befixed in all the fss. I am on S1S 5 so I believe it is based on NetBeans 3.5 I think what is going on for me is that the property sheet is cached. I don't seem to be able to get it to recreate. Note that I am looking at the method node and its property sheet. Ok, I was looking at the JavaDataObject node, which behaves in the way I described above. You are right that the SourceElement Nodes are not refreshed. I will try to find out a way how to solve the problem, however the FileChange event is not a solution for now, it would require quite big changes both in CVS and OpenIDE. Is there any progress on this bug? Also, I think this bug should be a P2 and given more attention because it presents to our users a serious usability problem. This is impossible to fix in VCS modules. There's no way how we can inform anyone about the attribute change. And it's not even possible to reliably detect external modification of the attribute. This needs to be solved in Java module. I raised this bug back to P2 because I think this is a serious problem and more attention needs to be paid to it. Ok, we pay attention to this problem, but there is no nice solution of it. The CVS fs (and any other fs, which uses java.io.File) is not able to detect the change due to reasons I have described above. The only way for the Edit command is to remember old metadata, perform the command and compare new metadata with old ones. This could be incredibly slow. Java MemberElement's properties should test the file, if it is read only or not. But still the problem will be caching of properties. Just curious, what are the criteria for throwing out cached properties? Is there any way to programmatically thrown them out? FWIW, in the long term this is an issue that should be addressed by JSR203, which will add async IO support at the Java platform issue so native code for this isn't necessary. Yes, there is a way how to throw out the cached properties (firing property change event), but the Java node is not notified on the change of File attributes, because file system does not detect it and does not fire fileChange. For Java module there are two possible solutions: 1. Either poll on FileObject for change, very awful. 2. Listen on ExplorerManager for node selection, and in this case check the file attributes, this solution will require user to reselect the affected Java node. > because file system does not detect it and does not fire fileChange
this is correct, but the VCS edit command can detect it. As Tomas
Zezula indicated in his comment the module can remember the attributes
of all files which can be changed in one way or another by the
external command. After the command finishes it can compare the
attributes and fire the change events properly. I don't think we need
to change any persistent data format, all can be done in memory. Note
that the command usually involves network operation which is _much_
slower than querying the local disk anyway.
For a short-term workaround, I think the solution of requiring the user to reselect the node is acceptable and the easiest to implement. All we really need is a predictable way for the cached properties to be thrown out and recreated. The rest is just a matter of documentation. Of course, I like Trung's suggestion too but it only works for VCS-mounted filesystems. Peter Liu: you're probably referring to Tomas Zezula's suggestion 2. Listen on ExplorerManager for node selection, and in this case check the file attributes, this solution will require user to reselect the affected Java node. I must warn loudly that this potentially will slow down navigation in the explorer, thus create a very visible UI responsiveness problem Added pnejedly to CC list Created attachment 9567 [details]
Proposed patch
The attached patch does two things: 1. creates properties always in r/w state, unless the caller passed "fale" to writable parameter of the Node's constructor (e.g. from Clazz module). 2. when the code generator is about to modify the source, it explicitly acquires file lock. This should produce an IOException on local filesystems, which is then turned into SourceException and propagated to the caller. Without this action the property sheet accepts edit (because the underlying model happily writes into Document over r/o text) and the change is then silently undone by Editor module. I will create binary patches for testing on VCS and attach them to this issue. Created attachment 9568 [details]
Binary patch for testing
Unzip the .zip into the installation directory (it should creat dirs modules/patches/* and modules/autoload/patches/*) Patch seems fine for me. Patch seems to be ok from QA point of view. Dane, can you check it with VCS edit command? Yes, I'm going to check it.... please wait well, here's my observation: ============================= I have r/o file on disk (in G-CVS FS) When I open this file for edit, File is opened in editor and in it's TAB is writen "bla [up-to-date; 1.1] [read only] any attemt to write in this file via editor invokes Question dialg: "Do you want to run the Edit command to make the file writable?" if you press YES, then the file change on the disk to r/w (for user only) nad I'm informed by Information dilolg that file bla.java is prepared for edit... There is still remain [read only" in the file edior's TAB, but it deasmis when you write first char into editor. ------------------------------------------------------- But if you have r/o file (DataObject) and slecet just its main node in the Explorer. It has 3 properties and one of them is disabled. It is the Name property. If you expand its nodes (class, Fields, Constructors, Methods, ...) and for instance select node "class bla" and click on (enabled) property Name and start to edit-> I over-wrote its value "bla" to "aaa" and hit enter... I've got Information dialog: "Invalid value. The property Name could not be set. Reason: Do you want to run the Edit command to make the file writable?" and only OK buton is available (probably 'cause it is Info.dialog:) After clicking on OK, nothing is change (on the disk), no cvs->Edit is executed and all remains is it was (I mean... read only) -------------------------------------------------------------------- OK - I will add a special handler for UserQuestionException (which is the thing that you saw in the dialog); if the action is confirmed, the property setter will retry the action (this time with writable file). Stay tuned for another patch. Unfortunately, UserQuestionException.confirmed(), which is thrown from VCS filesystem when the file is not writable does not wait for the VCS edit command's completion - please see issue #23407. That leads to errors when the property impl. retries the modify operation immediately after the user confirms the question dialog. Martin E. has suggested that the other issue could be possibly fixed for 3.5 in a trivial way. Today Svata provided me a binary patches and I tested them o ntodays NB35dev #200303312350 on my linux RH72. Well, On the first attempt everything seems work fine, but then I tried it again, and problerms occured:-( Here's my reproduction as far as I remember it right:-/ ======================================================= I've set everything just for testing (G-CVS mounted, r/o files in it) 1. I expanded node of class "bdfgdf" or what was it's name and choose a node of class. In the property I change this class name to say "Hov". The Question Y/N dialog occured asking something like this: "Do you want execute Edit command to make this file r/w ?" and YES. Then the Info dialog appered: "The file "bfdfg.java" is prepared for edit. I'd like to point that in this time no file "bfdfg.java" exists on the disk, but new file called Hov.java took its place. Which isn't also in cvs repository and thus should be [Loca]. In the Explorer it is [Locally Modified] :-( Press OK. 2. Now, I wish test the second way. And maybe my following steps are not so logical, if you will follow them. But leads to troubles :-/ Some E were thrown and ide freez :-( I can't found a reproduction..... but IMHO ide shouldn't freez during any bad user action. Attaching a file with FTD and exception printed into console. Created attachment 9638 [details]
FTD +E. stackTraces
And if I test it repeatedly with checkouting r/o the same file and trying to write into some of its property related to the class/field/constructors/ etc. nodes and answering sometimes yes and sometimes no.... I was surpriced that the Question dialog is someties only Y/N and sometimes Y/N/C Some other observation is if I try change Parametrs of constructor and answering NO for execution of Edit command.... The Parametr is remebered even thouh I change focus to different file/DO in the Explorer (in the same package) I think I have a reproduction for deadlock: =========================================== Have a r/o file and have expanded its nodes in Explorer. 1. Choose a children node of Constructors. 2. Change property "Parameters" to something different (in my case I was changing def. constructor to accept String nic) 3. The Q. Y/N or maybe Y/N/C dialog appears.... 4. From now answer always NO. and comfirm Info dialog. by OK 5. Now repeat from step 2 a few times ( try to use different values) 6. And now say YES. and ide will freez.... attaching a new FTD Created attachment 9639 [details]
another FullThreadDump
There's not my code in the dealock, sorry. My guess is that the document become modified somehow (?), VCS commands tries to save it, a dialog is displayed, but AWT thread waits on confirmed() completion => deadlock. No idea why the file become modified; the code generator attempts to lock() the file before it even starts to modify the document. Perhaps I could eagerly dig out the file in the runAsUser() and try to lock() it in advance, but I do not think it would fix this particular deadlock (takeLock() [in awt] -> VCS command exec -> editor save -> deadlock). I didn't tell the deadlock is in your code or else where, but it is here...:-/ Do you think it's more like a new bug for vcsgeneric? I need to investigate how the file become modified; it should not (if you answered "No" all the time) I've looked very briefly into the latest thread dump. The confirmed() method should not be executed in AWT. I will block now until the command is finished (after the fix of issue #23407). It's not acceptable to block the AWT. This will solve the deadlock, because VcsManager.showCustomizer(..) must not be called from AWT thread (I didn't found a workaround yet for this requirement). Good; then the most proper fix would be to fork org.openide.explorer.propertysheet.PropertyPanel$WriteComponentListener.setAsText to a different thread (in openide's property sheet) - or fork confirmed() and run a local event loop ? Petre, do you think that making the value change asynchronous in PE.setValue() is acceptable (if so then the PE has to do its own error reporting, since it cannot throw an exception to the caller). BTW please javadoc that UserQuestionException.confirmed may be dangerous to call from a GUI thread. "Petre, do you think that making the value change asynchronous in PE.setValue() is acceptable (if so then the PE has to do its own error reporting, since it cannot throw an exception to the caller)." Question for me? I don't know much about this topic, but generally, the user itself is asynchronous so the PE can't recognize it is asynchronous, right? Or you are talking about possible inconsistency between the UI and the PE (and possible problems with proper rollback in case of some failure)? Yes, I asked whether it is acceptable to call setValue() from a non-GUI thread by the property sheet - I don't know much about issues within property sheet infrastructure. If I see the thread dump right, then it is not possible to call UQE.confirmed() from an AWT thread (unless Martin removes the limitation, if it is possible at all). Since the PropertyEditor.setValue() is required to change the state or throw an exception, I should: a) wait for the confirmed() => deadlock if setValue() is called from a GUI thread. PropertySheet may replan setValue() call, which removes the necessary condition for the deadlock. b) schedule the work with confirmed() and operation retry to another thread: removes the necessary condition for the deadlock to occur, but requires to add additional error reporting (fire property change back to the original state + dialog display). c) find out *why* the editor/data object gets (IMO incorrectly) modified. This is the weakest solution, actually, because the file may be marked r/o externally while already modified in the Editor. d) avoid displaying VCS dialogs when AWT waits for VCS command completion - if possible at all. What are your preferences ? > If I see the thread dump right, then it is not possible to call > UQE.confirmed() from an AWT thread (unless Martin removes the > limitation, if it is possible at all). Well, I'm a little confused here. Either I will remove the waiting for the command (rollback of issue #23407), do everything asynchronous and you can keep calling UQE.confirmed() from an AWT thread. But I've fixed issue #23407 because I supposed that you should know when the command, that change the RW status is finished. I guess you do not want the command to be run in AWT, or AWT to be waiting for the command to finish. Thus you must not call UQE.confirmed() from AWT. It's not guaranteed, that UQE.confirmed() finish in a short time. It may take several seconds, but it may even not finish at all if the command blocks somewhere. So, please do not call UQE.confirmed() from AWT, the code which is executed there must not run in AWT anyway. > avoid displaying VCS dialogs when AWT waits for VCS command > completion - if possible at all. I agree, that this would be good to solve. However, it's not nice to call it from AWT anyway, since it can take some time. The UI responsiveness will be frozen for some time. You're right, that this should be documented in javadoc of UQE.confirmed(). This looks like something that should be release noted (at least for the beta). Martin, Tomas, or Svata, is this text correct? Also, is there any user workaround? "Description: If you use the VCS Edit command to make a read-only file writable, the property sheet does not become writable." I'm afraid I'm even able to reproduce this deadlock on 1CPU kernel 2.4.18-27.8.0 on my DELL Latitude C840. I'm gona attach the FTD. FYI: the reproduction or steps which leads to deadlock probably couldn't be pronaunced as common user workflow. The reproduction is that you repeatedly do the folowing till you got Q Y/N/C dialog (normaly you\ve got only Y/N).: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Expand DO node to its constructor node. (Suppose it is done on r/o file!). Change its parametrs property. And doing repeatedly answering NO to all qestion about make it writtable. Also try to modify DO's fields its value and Type... You must once got Y/N/C Question dialog. After you answer on such dialog, notice please, that tha last change is persitant!!! Then try to change any of DO's properties and say YES, like you wish to make the file writtable and you've got the deadlock. Yes, it is what normal user would never done.... Created attachment 9702 [details]
Full Thread Dump on 1 CPU kernel
A question regarding the Y/N/C dialog: are the message and its title within exactly the same (verbatim) as for the usual Y/N dialog ? If not, could you please, shot a thread dump at the time the unusual dialog is displayed ? I am asking because the added code displays Y/N dialog unconditionally; there should not be any Y/N/C dialog at all. Can not reliably reproduce, it seems as a race condition. [Svata] Investigation showed that a first call to MultiDataObject.Entry.takeLock succeeded, taking a lock (!) instance - this was java module's check for writability. Then the code generator changed text and that called takeLock() again, this time throwing an exception. What is known for sure is that the Y/N/C dialog is displayed by the Editor (CloneableEditorSupport) from markModified() handler as a response to a UserQuestionException. That dialog is displayed from the default RequestProcessor. We weren't able to reproduce this condition (leading to the deadlock) during long repeated testing. Downgrading priority to P3, the main part will be fixed for 3.5, except the deadlock for which the issue remains open. Not release noting for now, due to the apparent rarity and randomness of occurence. If somebody disagrees, please speak up and re-add the RELNOTE keyword. We've agreed I enter a new issue #32628 for the deadlock with P3 priority and this I put again P2. This would help to close this issue as FIXED and have an extra issue for the deadlock. Tom, then you could integrated (if you'll get an approve of'course:-) and mark it as fixed. thanks removing RANDOM keyword which is related to the deadlock issue #32628 now Integrated into netbeans35 branch. Verifying on S1S5 #030406 Removing S1S5_WAV keyword. Bug already fixed and integrated into release35 branch by Tomas Zezula. Adding Jan Rojcek on cc. This is the issue that has changed the behavior of property sheet when a file is read-only using the UserQuestionException and waiting for the VCS command in AWT thread. I've discussed the associated deadlock problems (issue #32628) with Jan and the conclusion was, that this is not a really nice solution, because the user is blocked. Probably the only way to solve the deadlock is to implement a modal dialog that will handle the command output (solution (c) in issue #37596). But perhaps a better contract should be defined, because the value set by the user is lost if the command fails. Perhaps there should be Perhaps we should define some APIs for firing changes of the writeable state of files. Like FileAttributeEvent("read-only"). However, the implementation will be complex. |