Community
Participate
Working Groups
I am using 3.1.2. Basically a new install less that two days old (old workspace). This is what I did; I am adding public to a bunch of methods in a certain package. 1. I selected all the classes in this package and right click then chose open. 2. starting from the top, double click the file (which selects it in the editor). 3. starting from top of file, scroll down, paste in "public ". scroll some more, paste again. 4. push save. Then go to next file. I guess im pushing close sometimes as well. this worked for about 4-5 files in a row then failed. And worked for several more, then failed again. So of all the editors I opened, two refuse to save their changes. These files have the change * but it won't save. No errors are thrown. refreshing workspace does nothing. I can undo/redo and the * will come and go. I can save other files. There are java errors in the file as well. I can double click in the project view which causes the file to be selected in the editor view. Still cant save. Its basically like the save function is detached from the editor some how. The action is there because if i undo/redo the save button will enable/disable. I just clicked to close one of the files. It opens the save changes dialog. I select to save, but the changes are just silently discarded. Reopening the file with the Java Editor and editing it now saves the contents properly. This time i right clicked the other file that was not saving and selected "open with text editor." It opened and automatically got the *. I pushed save on the Text Editor, and now its saved for both editors. I can make a change and both editors will realize the file needs saving. But the Java Editor still won't save the file. Also eventhough it lost the * and disabled the save button in the Java Editor after I saved with the Text Editor, the error icon is still present in the java editor. This could be related to the rapid opening of so many files at once by shift-click selecting then choosing open. I also had this happen to me about a year ago with 3.1.1.
I cannot reproduce this. Please provide more detailed reproducible steps and also explain what "fail" means (dialog, .log entry, ...)?. See also: http://dev.eclipse.org/viewcvs/index.cgi/%7Echeckout%7E/platform-text-home/development/bug-incomplete.htm
Faile means, "Java Editor won't save file" nothing more, nothing less. The procedure I will be using to reproduce is; 1. to select about 20-30 java files, then right click and select open. 2. make simple change in a file then push save. 3. see if the save * has disappeared. 4. go to next file. Can you point me to where the Java Editor Save code is? And how does the editor know the file is saved? I know where in the process the save is failing, but I need to match that to a piece of code hopefully. I think the easiest strategy for me will be to find the general area involved, then look for ways the save can fail without throwing any user visible exceptions. facts I know; 1. Java editor sometimes wont save a file. 2. nothing I do in the java editor causes the file to save. 3. Opening same file with text editor, while its still open in java Editor, then saving in text editor, saves the file. 4. Then the Java Editor recognizes the file has been saved and removes the * and disables the save button, but it does not then process the file changes and clean up the errors. Normally when a Java file is changed through a text editor, or external modification, after the change the java editor eventually checks for java errors again. 5. Problem disappers usually when JavaEditor for the file is closed. So I change my earlier position, clearly the action is working. Its the save witin the Editor that is not.
OK, I got it to happen again. This time I have the debugger on it. The difference is in org.eclipse.jdt.internal.core.CommitWorkingCopyOperation line 94 IBuffer primaryBuffer = primary.getBuffer(); For the ones that save properly this returns an instance of org.eclipse.jdt.internal.ui.javaeditor.DocumentAdapter For the ones that fail to save that line returns an instance of org.eclipse.jdt.internal.core.Buffer This is as much as I can find out now before I pass out...
>2. make simple change in a file then push save. - Push save? Where? main menu? Ctrl+S? The Eclipse SDK does not have a Save button i.e. nothing to push ;-) I assume you use the File > Save menu, context menu > Save or Ctrl+S, probably Ctrl+S, right? Check the enablement state of File > Save and context menu > Save and try to save using them. It might be a key binding problem. It would be great if you could continue to debug this since we cannot reproduce it here. The 'Save' entry point in the Java editor is CompilationUnitditor.doSave(IProgressMonitor) Can you confirm that you don't have other plug-ins installed and that the files are opened by the Java editor?
The save "button" since typically buttons are pushed ;) Its right at the top next to the file new button I believe. No I didnt use the menuitem or ctrl-S, but I meant too. It was too late and I forgot to do it being sleepy. If I can get it to happen again, ill do that. But either way as you can see in the prior post, the save action was actually being triggered. So I don't think we have a key binding issue or an issue with the action either. The only feature I added on top of the base workbench is GEF. Yes I can confirm that the files are opened by the JavaEditor. I can further confirm that this is a JavaEditor because I saw the class as I stepped through the debugger. I stepped through the compilationunit.doSave point and into the CompilationUnitDocumentProvider that actually performed the save. At no point was the save aborted. As I stated in post #3, the only difference between a working session of Java Editor and a non-working session of Java Editor was the type of IBuffer returned by primary.getBuffer(); What do you think about my comments in post #3? Is JavaEditor supposed to return an org.eclipse.jdt.internal.core.Buffer?
>The save "button" since typically buttons are pushed ;) Sorry, forgot about that one since I never use it. >What do you think about my comments in post #3? Is JavaEditor supposed to >return an org.eclipse.jdt.internal.core.Buffer? This would indeed be a bug and explain why it is not saving the file. The Buffer should only be used for - non-primary working copies - for class files (but of course they can't be saved) Can you reproduce this without GEF i.e. download Eclipse SDK into an empty directory and then try again? If so, please put a breakpoint into the Buffer constructor to see who creates that buffer.
Well since I am using a runtime workbench to debug, I'll disable GEF from the launch configuration. I thought about putting a breakpoint in the Buffer and I will give that a try. But it appeared that the buffer was default for all until the other type got set. So it appears that somehow some part of the program is forgetting to set the correct DocumentProvider. Ill investigate further. Also, the working copy was primary. At least I think so because there were some isPrimary checks and whatnot and it always passed as being primary.
Also, can you reproduce this on 3.2 RC7?
I cought the thread red-handed! The method BufferManager#addBuffer(IBuffer buffer) on the line this.openBuffers.put(buffer.getOwner(), buffer); I put a conditional break point with org.eclipse.jdt.internal.core.Buffer.class.isInstance(buffer) as the condition. I kept opening and closing sets of files from different packages and enabling the BP on open, and disabling the BP on close until I got the BP to trigger on one of the files that was being opened. The following is the stack trace Thread [org.eclipse.jdt.internal.ui.text.JavaReconciler] (Suspended (breakpoint at line 57 in BufferManager)) BufferManager.addBuffer(IBuffer) line: 57 CompilationUnit.openBuffer(IProgressMonitor, Object) line: 1020 CompilationUnit.buildStructure(OpenableElementInfo, IProgressMonitor, Map, IResource) line: 104 CompilationUnit(Openable).generateInfos(Object, HashMap, IProgressMonitor) line: 233 SourceType(SourceRefElement).generateInfos(Object, HashMap, IProgressMonitor) line: 114 SourceType(JavaElement).openWhenClosed(Object, IProgressMonitor) line: 488 SourceType(JavaElement).getElementInfo(IProgressMonitor) line: 232 SourceType(JavaElement).getElementInfo() line: 218 CancelableNameEnvironment(SearchableEnvironment).find(String, String) line: 120 CancelableNameEnvironment(SearchableEnvironment).findType(char[], char[][]) line: 188 CancelableNameEnvironment.findType(char[], char[][]) line: 45 LookupEnvironment.askForType(PackageBinding, char[]) line: 119 PackageBinding.getTypeOrPackage(char[]) line: 178 MethodScope(Scope).getTypeOrPackage(char[], int) line: 2455 MethodScope(Scope).getType(char[]) line: 2194 SingleTypeReference.getTypeBinding(Scope) line: 39 SingleTypeReference(TypeReference).resolveType(BlockScope, boolean) line: 124 SourceTypeBinding.resolveTypesFor(MethodBinding) line: 1270 SourceTypeBinding.methods() line: 1009 SourceTypeBinding.faultInTypesForFieldsAndMethods() line: 615 CompilationUnitScope.faultInTypes() line: 405 CompilationUnitProblemFinder(Compiler).resolve(CompilationUnitDeclaration, ICompilationUnit, boolean, boolean, boolean) line: 561 CompilationUnitProblemFinder(Compiler).resolve(ICompilationUnit, boolean, boolean, boolean) line: 607 CompilationUnitProblemFinder.process(CompilationUnitDeclaration, ICompilationUnit, char[], Parser, WorkingCopyOwner, IProblemRequestor, boolean, IProgressMonitor) line: 165 CompilationUnitProblemFinder.process(ICompilationUnit, char[], WorkingCopyOwner, IProblemRequestor, boolean, IProgressMonitor) line: 214 ReconcileWorkingCopyOperation.executeOperation() line: 79 ReconcileWorkingCopyOperation(JavaModelOperation).run(IProgressMonitor) line: 718 ReconcileWorkingCopyOperation(JavaModelOperation).runOperation(IProgressMonitor) line: 777 CompilationUnit.reconcile(int, boolean, WorkingCopyOwner, IProgressMonitor) line: 1081 JavaReconcilingStrategy$1.run() line: 98 InternalPlatform.run(ISafeRunnable) line: 1044 Platform.run(ISafeRunnable) line: 783 JavaReconcilingStrategy.reconcile(boolean) line: 82 JavaReconcilingStrategy.initialReconcile() line: 174 JavaCompositeReconcilingStrategy(CompositeReconcilingStrategy).initialReconcile() line: 114 JavaCompositeReconcilingStrategy.initialReconcile() line: 120 JavaReconciler(MonoReconciler).initialProcess() line: 103 JavaReconciler.initialProcess() line: 328 AbstractReconciler$BackgroundThread.run() line: 170 I'd rather nail it down where I have it first as I just installed 3.1.2 last week. And this bug I did report a long time ago against 3.1.1. So I hesitate to install 3.2, but wont mind after it gets root caused.
Created attachment 44278 [details] Trace of improper addition to BufferManager This is the trace where instead of the proper Document type being added to the BufferManager, a "Buffer" was added.
Created attachment 44279 [details] Normal path trace Normal addition of DocumentAdapter to BufferManager
Jerome, any idea what might happen here?
The trace from comment 9 doesn't indicate anything wrong. A DocumentAdapter buffer is not expected in this case since it comes from the compilation unit lookup that is not a primary working copy. So sorry, no idea of what could be wrong without steps to reproduce. Also I'm not sure about the value of trying to debug it on 3.1.2 if it is fixed in 3.2 RC7 (and no 3.1.3 being planned).
What a frustrating comment. 1. You were given steps to reproduce. I have demonstrated several times that I can reproduce it this way. Also, I am using a flash drive, so the files open a bit slower than they might from a harddrive if that matters. 2. The problem is not the trace, but when it happens. Since this is a thread we don't have direct information about what programmatic sequence set this thread going. But you do have the procedure I was executing at the time. You think its appropriate for this thread to do what it did when it did it? I don't understand this code but to me it looks like this background thread ran much later than it should have and probably overwrote the existing IBuffer which was a proper DocumentAdapter with this new IBuffer which was a Buffer. From what I have seen, Buffers overwrite DocumentAdapters when the java file is closed, not when its opened. What reason do we have to believe that this won't happen in 3.2? Any insight would be appreciated.
OK I found the bug. It's a timing issue when two threads access the CU and want to get its buffer. Steps I was able to reproduce in 3.3 stream (but not constantly): 1. put a breakpoint into BufferManager.addBuffer(...) 2. start to Debug a target 3. import JUnit into your target 4. open ClassLoaderTest ==> breakpoint will be hit in main thread 5. resume ==> breakpoint will be hit in reconciler for TestCaseClassLoader which it seems to open when reconciling ClassLoaderTest 6. DO NOT RESUME the reconciler thread 7. open ClassLoaderTest ==> breakpoint will be hit in main thread 8. resume the main thread ==> DocumentAdapter is registered as buffer since ClassLoaderTest is now a working copy 9. resume the reconciler ==> BUG: a Buffer instance is now registered as buffer for the ClassLoaderTest working copy. Save now no longer works since it goes to the Buffer instead of the DocumentAdapter
JDT Core should either protect the buffer manager or the CompilationUnit from overwriting a working copy's buffer.
woot!! \o/
Thanks Dani. These are very good steps. I'll investigate.
Created attachment 44985 [details] Proposed fix Fix consists in testing if a buffer already exists for the compilation unit inside a synchronized block on the buffer manager.
The part I didn't like about the situation was that the Reconciler thread was sharing in the state manipulation of BufferManager. Neither thread can know for sure what type of IBuffer it will be getting back since it can't know what the other thread is doing. Which is ok if its abstracted away. The patch seems to address the symptom of the issue. It prevents the Reconciler thread from doing what its trying to do. But it does not stop that thread from _trying_ to do it. I think that thread and perhaps even Main need to understand the state of the BufferManager before they try to perform these actions. Also lets consider that the IBuffer is removed from the BufferManager. Since the Reconciler tries to add buffers, then resonably it should also remove buffers lest it leave buffers out there inappropriately. This also will require synchronization. Either way remove requires synch since add is performed by multiple threds. Moreover, note that the BufferManager uses an overflowinglrucache which uses a map that is not synchronized yet we are accessing it by multiple threads. The LRUCache explicitely states that it is not thread safe. Its a bit much to understand the architecture enough to see where the change needs to be. But I suspect openWhenClosed since the 'whenClosed' part can't really be known currently AFAICT.
Unfortunately, each time we call out to client's code (e.g. IBuffer#close(), WorkingCopyOwner#createBuffer(...)), we have to be very carefull not to do this inside a synchronized block/method. Indeed experience shows that this calls for deadlocks as we don't control the client's locks. For these reasons, we cannot protect the addition/removal of buffers (this can cause IBuffer#close() to be called) or we cannot prevent 2 threads to open an ICompilationUnit at the same time (this causes WorkingCopyOwner#createBuffer(...) to be called). This is why the patch only ensures that what is put in the cache is consistent. If you find a case where the addition/removal of a buffer in the buffer cache causes another inconsistency, please open a separate bug report.
Patch released for 3.3 M1 in HEAD.
Code verified for 3.3 M1 using build I20060807-0010. Unfortunately I could not reproduce bug with 3.2 build.
