Community
Participate
Working Groups
This is a "reopening" of Bug 77564 but I was not the original creator of that bug so I can not just re-open it. I do not know why the originator of Bug 77564 did not re-open the bug when asked if it was still a problem, because it is certainly still a problem. The issue is still the same. JSP tools uses a SearchParticipant in an attempt to get results from JSP documents in Java searches and re-factorings but the existing API does not seem to be enough. SearchParticipant has the method #selectIndexes and part of its documentation says "An index location represents a path in the file system to a file that holds index information." and to that effect we store our .index files in our own location and return those when the #selectIndexes method is called using the BasicSearchEngine. But the BasicSearchEngine uses this internal JDT "PatterSearchJob" which has a #getIndexes method which does the following: 1. Asks the participant for the index location paths in this case from our JSP SearchPartiicpant 2. attempts to get Index files from the JavaModelManager using these paths this second step of course does not work because our index files are not registered with the JavaModelManger. There also appears no API to register our index paths with the JavaModelManager. We have attempted to use internal API to register our index files with the JavaModelManager to get around this issue but whether its because we are using the "wrong" internal API (as if there is a "right" internal API) or because the JavaModelmanager was just not designed to accept index files from outside of the place it normally stores them, this internal API approach only works spottily. In my attempts to get this working using the internal API I have tried using the IndexManager#ensureIndexExists method and other like methods to try and inject our index into the Java IndexManager. But I get rather strange behavior sometimes where it seems the IndexManager starts using the JSP index to store Java indexing for a specific location. I assume this is because our index file is trying to register for the same container path as the Java index file. Along thous lines I have tried to use different location for the containerPath when registering our JSP indexes, using everything from the project path, to a folder path containing JSP files to random strings (trying to avoid clashing with the container paths registered by the Java index files). None of this seems to work or if it does it does not work reproducibly due to the already described strangeness that goes on with conflicting index locations for the same container path. So at the end of the day what am I asking for. The API seems to be in place to contribute to the Java search through the use of the Searchparticipant class and either one of two things is true. Either we are using the API incorrectly and we need further clarification of the documenation. Or the API is incomplete in that there indeed a conflict in that the SearchParticipant class allows us to return an index location of our choosing but the PatternSearchJob which is used by the BasicSearchEngine always wants the indexes to be in the location chosen by the Java IndexManager. I appreciate any further information or help that can be given in this manor.
Has there been any further investigation into this yet? Thanks for the update.
(In reply to comment #1) > Has there been any further investigation into this yet? Thanks for the update. Unfortunately no :-( This is to late in the 3.6 game to have a look at such bugs. If, early 3.7, you will not see any change on this request, then do not hesitate to ping us again...
Are the indexes getting created through SearchParticipant#scheduleDocumentIndexing()?
(In reply to comment #3) > Are the indexes getting created through > SearchParticipant#scheduleDocumentIndexing()? We are not currently overriding SearchParticipant#scheduleDocumentIndexing() in our SearchParticipant but we do call #scheduleDocumentIndexing passing in our implimentation of SearchDocument and the location of our index file.
(In reply to comment #4) > We are not currently overriding SearchParticipant#scheduleDocumentIndexing() in > our SearchParticipant but we do call #scheduleDocumentIndexing passing in our > implimentation of SearchDocument and the location of our index file. You don't have to override scheduleDocumentIndexing() but should be calling to really get the indexes register with the JavaModelManager. In the JUnit test case we have, I could make sure that calling scheduleDocumentIndexing() was being good enough. Can you make sure that the call is happening properly in your case? If not, could you give me reproducible steps?
(In reply to comment #5) > You don't have to override scheduleDocumentIndexing() but should be calling to > really get the indexes register with the JavaModelManager. Right now we are calling JSPSearchParticipant#scheduleDocumentIndexing(JavaSearchDocumentDelegate, pathToIndexFile) where JSPSearchPartiicpant is our implimentation of SearchParticipant and JavaSearchDocumentDelegate is our implimentation of SearchDocument for JSP files so the path returned is actully one to our translated java file. > In the JUnit test > case we have, I could make sure that calling scheduleDocumentIndexing() was > being good enough. Can you make sure that the call is happening properly in > your case? Not sure what you mean by "properly". > If not, could you give me reproducible steps? Not sure what to give you here either. The side effect for us is that if we open a new workspace and add a bunch of JSP files and index them all, which includes calling #scheduleDocumentIndexing then our results will appear in Java search results. If you then close the workspace and open it back up Java searches will not find our search results unless we completely re-index everything and call #scheduleDocumentIndexing again.
(In reply to comment #6) > If you then close the workspace and open it back up Java > searches will not find our search results unless we completely re-index > everything and call #scheduleDocumentIndexing again I understand the problem now. The IndexManager is not completely persisting the information of the index files that can be reused. A workaround could be to call #scheduleDocumentIndexing for atleast one document in a project. This should ensure that the data structures in IndexManager to be updated properly and doesn't need the complete indexes to be created. However, this is not a good solution. There are mainly two reasons for this limitation -- 1. IndexManager currently persists only the index file names. It doesn't store the complete path and also doesn't store the container information. 2. If the IndexManager finds out the file is not there or finds out that it has to reindex for some reason, it doesn't have a way to ask the participant to reindex. The first limitation probably can be updated by just modifying the code or even by using the workaround, but to have a better story to include the second limitation, the API should be modified. I will look at the best way that could be done.
(In reply to comment #7) > I will look at the best way that could be done. Thank you for continuing to look into this, it is very much appreciated.
Satyam, Any news to report? Thanks again for the investigation thus far, it is great to see some movement on this.
(In reply to comment #9) > Satyam, Any news to report? Thanks again for the investigation thus far, it is > great to see some movement on this. As I mentioned, the best way I think should be is to introduce a new API SearchPaticipant#initializeIndex(IPath indexLocation), which will return true if the file is good and JDT could read that file properly. If it returns false, it means that the JDT couldn't make use of the file and hence scheduleDocumentIndexing() had to be called once again. Is that good for you? Do you need a fix for previous versions of Eclipse? Frederic, What do you think about this API proposal?
(In reply to comment #10) > As I mentioned, the best way I think should be is to introduce a new API > SearchPaticipant#initializeIndex(IPath indexLocation), which will return true > if the file is good and JDT could read that file properly. If it returns false, > it means that the JDT couldn't make use of the file and hence > scheduleDocumentIndexing() had to be called once again. > > Is that good for you? Do you need a fix for previous versions of Eclipse? > > Frederic, What do you think about this API proposal? I didn't mention it completely in my last comment. This API should be called for all the index locations at the startup. This will register the index files with the IndexManager.
(In reply to comment #10) > (In reply to comment #9) > > Satyam, Any news to report? Thanks again for the investigation thus far, it is > > great to see some movement on this. > > As I mentioned, the best way I think should be is to introduce a new API > SearchPaticipant#initializeIndex(IPath indexLocation), which will return true > if the file is good and JDT could read that file properly. If it returns false, > it means that the JDT couldn't make use of the file and hence > scheduleDocumentIndexing() had to be called once again. > > Is that good for you? It sounds good. I won't know for sure until I actually get my hands on it and can test it in our situation. > Do you need a fix for previous versions of Eclipse? We would like this in 3.6.1 if at all possible so that we have it for use in WTP 3.2.2
(In reply to comment #11) > (In reply to comment #10) > > As I mentioned, the best way I think should be is to introduce a new API > > SearchPaticipant#initializeIndex(IPath indexLocation), which will return true > > if the file is good and JDT could read that file properly. If it returns false, > > it means that the JDT couldn't make use of the file and hence > > scheduleDocumentIndexing() had to be called once again. > > > > Is that good for you? Do you need a fix for previous versions of Eclipse? > > > > Frederic, What do you think about this API proposal? > > I didn't mention it completely in my last comment. This API should be called > for all the index locations at the startup. This will register the index files > with the IndexManager. I'm a little bit afraid by adding an API which should be called at each startup otherwise it would not work properly. IMO, but maybe Im' wrong, I do not think an additional API is really necessary. I'm thinking about storing the location into the table when testing whether the index does exist or not (typically in JMM.ensureIndexExists(IPath, IPath) method when state == null)... Wouldn't it do the trick? As soon as the location would be in the table, then we could benefit of the JMM mechanism at startup to retrieve index files and the client specific ones would be taken into account without any other additional changes... Do I miss something or could that work?
Moreover, adding an API would be a NO GO for a change in 3.6.1 version, hence it would be better to find a non-API change to fix this issue...
(In reply to comment #13) > I'm a little bit afraid by adding an API which should be called at each startup > otherwise it would not work properly. > > IMO, but maybe Im' wrong, I do not think an additional API is really necessary. > I'm thinking about storing the location into the table when testing whether the > index does exist or not (typically in JMM.ensureIndexExists(IPath, IPath) > method when state == null)... Wouldn't it do the trick? As soon as the location > would be in the table, then we could benefit of the JMM mechanism at startup to > retrieve index files and the client specific ones would be taken into account > without any other additional changes... > > Do I miss something or could that work? I tried to explain that in comment 7, but I think it is not that clear. You are right, this problem can be fixed by just adding it appropriately. However, if the index version changes or the index file doesn't exist, there is no way to tell the caller that a re-index needs to be done. Hence, I thought an API will help to do that. We could do the fix as part of 3.6.1 and introduce the API in 3.7 - what do you say?
(In reply to comment #15) > We could do the fix as part of 3.6.1 and introduce the API in 3.7 - what do you > say? Understanding that no new API can go into 3.6.1 this sounds like the best solution and would at least put us in a better place for our WTP 3.2.2 release.
Created attachment 176958 [details] Proposed patch on HEAD
I will attach a patch for 3.6.1. shortly.
Created attachment 176961 [details] Proposed patch for 3.6.1 Frederic, Here is the patch for 3.6.1. Please review.
Sorry, but I have to reject this patch as there's an issue in SearchParticipant: the identity test at line 210 sounds invalid to me. IMO, it should be replaced by an equality test between the two paths. Another points in IndexManager: 1) getIndexes(...) a) I think that the participant index should also be rebuilt when the java like names extension has changed. b) why test if the index file exists and if so, then create the index instead of using the same method than when the containerPath is not null: index = getIndex(containerPath, indexLocation, true /*reuse index file*/, false /*do not create if none*/); Hence, IMO, adding a) and b) would lead to a change in this method which should only be something like: // only need containerPath if the index must be built IPath containerPath = (IPath) this.indexLocations.keyForValue(indexLocation); if (containerPath == null && !getJavaPluginWorkingLocation().isPrefixOf(indexLocation)) { // the index might belong to non-jdt search participant... containerPath = getParticipantsContainer(indexLocation); } if (containerPath != null) { // sanity check ... unchanged code here ... } 2) readParticipantsIndexNamesFile() a) one thing worry me in the fact that the participantsContainers can be null after having called this method... If there's no participant index files, the participant index names file does not exist, hence the participantsContainers will always stays null. Then, each time the getParticipantsContainer(IPath) method will be called, the readParticipantsIndexNamesFile will also be called. So, each time a new File will be created, Util.getFileCharContent will be called with this file, a FileNotFoundException raised and finally caught to exit the method without having done anything... :-( IMO, it would be really interesting in this method to initialize the participantContainers to an empty table and have a boolean saying that either the participant index names file does not exist or that it has been read... b) It seems that savedSignature is an unnecessary local variable c) The file itself could be store as a final field instead of having a static string PARTICIPANTS_INDEX_NAMES (like savedIndexNamesFile) When a) is taken into account, I agree this will be less interesting but I would prefer this implementation (it's just a personal feeling nothing mandatory in the new patch) 3) removeIndexPath(IPath) a) The added if block should be inside the block of the if (locations != null) statement. b) The added test should be rewritten after changes done in 2) c) I think the participant index names files should be updated when a key is removed from the participantsContainers table, shouldn't it? 4) updateParticipant(IPath, IPath) a) Shouldn't the participantsContainers table be refreshed when the index location already exists but the container path is different? I do not know if this is a concrete case, but I guess it might happen 5) writeSavedIndexNamesFile() a) The method may delete the file when there's no longer any participant container, but we can leave with an empty file, it doesn't hurt too much... Hope my remarks are clear enough...
(In reply to comment #20) Frederic thanks for the comments! I am just adding the comments for those I don't agree completely. > Another points in IndexManager: > 1) getIndexes(...) > a) I think that the participant index should also be rebuilt when the java > like names extension has changed. > b) why test if the index file exists and if so, then create the index > instead of using the same method than when the containerPath is not null: > index = getIndex(containerPath, indexLocation, true /*reuse index file*/, > false /*do not create if none*/); As I understand, the search participant may not be really worried about the java like name extensions. I think the files can be anything and hence I haven't really taken care of it. As we don't know how to really do a re-indexing as of now, I am ignoring the re-indexing part as of now. > 4) updateParticipant(IPath, IPath) > a) Shouldn't the participantsContainers table be refreshed when the > index location already exists but the container path is different? > I do not know if this is a concrete case, but I guess it might happen I actually implemented this way first and then removed :). Indexing story is a little weak here. The participant can use the same index file for two containers and we don't really handle that well in the other places. Hence I didn't handle that because it is arguable and here I get a better performance :) > 5) writeSavedIndexNamesFile() > a) The method may delete the file when there's no longer any participant > container, but we can leave with an empty file, it doesn't hurt too > much... I will leave it as it won't hurt:)
This is all looking really great, thanks a bunch. I have a general question related to the index. Currently in our implementation of SearchParticipant we have a different index file for each folder containing JSPs in the workspace. Looking at the JDT implementation it seems you use one index per project and JAR file. Would it make more sense to only have one index file per project rather then per folder?
Created attachment 177018 [details] Proposed patch for 3.6.1 Frederic, Here is the new patch updated with your comments.
(In reply to comment #21) > (In reply to comment #20) > > 1) getIndexes(...) > As I understand, the search participant may not be really worried about the > java like name extensions. I think the files can be anything and hence I > haven't really taken care of it. As we don't know how to really do a > re-indexing as of now, I am ignoring the re-indexing part as of now. > Sounds right, ok to keep your implementation > > > 4) updateParticipant(IPath, IPath) > I actually implemented this way first and then removed :). Indexing story is > a little weak here. The participant can use the same index file for two > containers and we don't really handle that well in the other places. Hence I > didn't handle that because it is arguable and here I get a better performance > :) > ok > > 5) writeSavedIndexNamesFile() > I will leave it as it won't hurt:) > Sure
(In reply to comment #23) > Created an attachment (id=177018) [details] > Proposed patch for 3.6.1 > > Frederic, Here is the new patch updated with your comments. I didn't see any changes for the remarks 2.a), 3.b) and 3.c) done in comment 20. As your comment 21 didn't say anything about these remarks, I was expecting to see the corresponding implementation in the patch. Do I miss something?
(In reply to comment #25) > I didn't see any changes for the remarks 2.a), 3.b) and 3.c) done in comment > 20. > As your comment 21 didn't say anything about these remarks, I was expecting to > see the corresponding implementation in the patch. Do I miss something? 2.a) Now, the participantContainers is not null at the end of readParticipantsIndexNamesFile() and this function is called only when the participantContainers is null. Hence, it should not be called once again. However, I am not using any other flag as I thought the null check and a get() should be good enough. I misunderstood 3.b. However, as I am not using any flag, I guess this is ok. 3.c. The participantUpdated flag should do the trick. Once this flag is triggered, saveIndexes() called during the idle time should write the participantfile. Do you think we should write it right away? I think it shouldn't be a problem.
(In reply to comment #26) > (In reply to comment #25) > > I didn't see any changes for the remarks 2.a), 3.b) and 3.c) done in comment > > 20. > > As your comment 21 didn't say anything about these remarks, I was expecting to > > see the corresponding implementation in the patch. Do I miss something? > 2.a) Now, the participantContainers is not null at the end of > readParticipantsIndexNamesFile() and this function is called only when the > participantContainers is null. Hence, it should not be called once again. > However, I am not using any other flag as I thought the null check and a get() > should be good enough. Ooops, sorry, I missed your change in this method... :-S You're right, with it, it's OK now :-) > I misunderstood 3.b. However, as I am not using any flag, I guess this is ok. Yes, again, with your change in readParticipantsIndexNamesFile() method 3.b is also OK now :-)) > 3.c. The participantUpdated flag should do the trick. Once this flag is > triggered, saveIndexes() called during the idle time should write the > participantfile. Do you think we should write it right away? I think it > shouldn't be a problem. The saveIndexes is called from notifyIdle(long) only when the needToSave flag has been set to true. As the removeIndexPath(IPath) does change this flag, there's a way where the participantsContainers table could be updated and the change not persisted in the corresponding file. Hence, IMO, we should write this file right away...
(In reply to comment #27) > > > 3.c. The participantUpdated flag should do the trick. Once this flag is > > triggered, saveIndexes() called during the idle time should write the > > participantfile. Do you think we should write it right away? I think it > > shouldn't be a problem. > > The saveIndexes is called from notifyIdle(long) only when the needToSave flag > has been set to true. As the removeIndexPath(IPath) does change this flag, > there's a way where the participantsContainers table could be updated and the > change not persisted in the corresponding file. Hence, IMO, we should write > this file right away... Obviously, you should read: "... As the removeIndexPath(IPath) does NOT change this flag,..." Sorry for the confusion
Created attachment 177073 [details] Proposed patch for 3.6.1 In this patch, I am calling the writeParticipantsIndexNamesFile() while removing the index.
(In reply to comment #29) > Created an attachment (id=177073) [details] > Proposed patch for 3.6.1 > > In this patch, I am calling the writeParticipantsIndexNamesFile() while > removing the index. I understood why I missed your change in readParticipantsIndexNamesFile() in previous patch... It's because the getParticipantsContainer(IPath) still had the null test on participantsContainers. Seeing that, I wrongly supposed that the method didn't change either... This patch fixes all this issue now and looks really good to me. Great work Satyam :-)
(In reply to comment #30) > (In reply to comment #29) > > Created an attachment (id=177073) [details] [details] > > Proposed patch for 3.6.1 > > > > In this patch, I am calling the writeParticipantsIndexNamesFile() while > > removing the index. > > I understood why I missed your change in readParticipantsIndexNamesFile() in > previous patch... It's because the getParticipantsContainer(IPath) still had > the null test on participantsContainers. Seeing that, I wrongly supposed that > the method didn't change either... > > This patch fixes all this issue now and looks really good to me. > > Great work Satyam :-) I have applied this fix to my workspace and it does not seem to fix my issue :( Do we need to do anything new to take advantage of this fix?
(In reply to comment #31) > (In reply to comment #30) > > (In reply to comment #29) > > > Created an attachment (id=177073) [details] [details] [details] > > > Proposed patch for 3.6.1 > > > > > > In this patch, I am calling the writeParticipantsIndexNamesFile() while > > > removing the index. > > > > I understood why I missed your change in readParticipantsIndexNamesFile() in > > previous patch... It's because the getParticipantsContainer(IPath) still had > > the null test on participantsContainers. Seeing that, I wrongly supposed that > > the method didn't change either... > > > > This patch fixes all this issue now and looks really good to me. > > > > Great work Satyam :-) > > I have applied this fix to my workspace and it does not seem to fix my issue :( > > Do we need to do anything new to take advantage of this fix? From the quick debugging I just did this is what I found: 1. in new workspace I created a dynamic web 2. created a Java class file with one method 3. created a JSP with a java region creating an instance of the java class and calling the method 4. do a references to search from the java class on the new method 5. result found in JSP 6. close workspace 7. open workspace 8. do search on same method, result in JSP not found 9. close project and open again 10. do search, result found while debugging I have noticed that in Index#query on any of the test cases I get no results from the disk index but on step 4/5 and step 10 results come from the memory index, on step 8 no results come from either the disk index or the memory index. Thus it seems to me that after shutting down the workspace our memory index must not be preserving itself to the disk index correctly and then when the workspace opens back up (step 7) there is no memory index yet because nothing has changed (like is caused in step 9 by opening and closing the project) and the disk index seems to be invalid, so I get no results. Suggestions?
(In reply to comment #32) > > From the quick debugging I just did this is what I found: > > 1. in new workspace I created a dynamic web > 2. created a Java class file with one method > 3. created a JSP with a java region creating an instance of the java class and > calling the method > 4. do a references to search from the java class on the new method > 5. result found in JSP > 6. close workspace > 7. open workspace > 8. do search on same method, result in JSP not found > 9. close project and open again > 10. do search, result found > > while debugging I have noticed that in Index#query on any of the test cases I > get no results from the disk index but on step 4/5 and step 10 results come > from the memory index, on step 8 no results come from either the disk index or > the memory index. > > Thus it seems to me that after shutting down the workspace our memory index > must not be preserving itself to the disk index correctly and then when the > workspace opens back up (step 7) there is no memory index yet because nothing > has changed (like is caused in step 9 by opening and closing the project) and > the disk index seems to be invalid, so I get no results. > > Suggestions? Some more debugging. I have found that our index is correctly being saved to disk on the workspace exit. But when the workspace comes back up and I do my first search (step 8) our index is being saved over by a blank index file. This means when the search actually happens nothing is in our disk index.
(In reply to comment #33) > > Some more debugging. I have found that our index is correctly being saved to > disk on the workspace exit. But when the workspace comes back up and I do my > first search (step 8) our index is being saved over by a blank index file. > This means when the search actually happens nothing is in our disk index. Sorry for the furry of comments, probably could have waited until I was done looking... So I found the problem. IndexbinaryFolder#exicute, when it gets to line 130 "this.manager.request(new SaveIndex(this.containerPath, this.manager));" the value of "this.manager" is: Enable count:1 Jobs in queue:3 0 - job[0]: indexing binary folder /Foo/WebContent 1 - job[1]: removing src/__2F_Foo_2F_WebContent_2F_bla_2E_jsp.java from index /Foo/WebContent 2 - job[2]: removing src/__2F_Foo_2F_WebContent_2F_Blarg_2E_jsp.java from index /Foo/WebContent .... Thous two Java files with the funky looking names were our temporary in memory Java translations of our JSP files. Probably is we don't ever save thous translations to disk so they are definitely not there when the workspace loads back up. We were counting on their indexed state being saved in the index and thus not having to actually keep the translated file around. But the JDT index is obviously detecting these Java translations are no where to be found and thus decides they need to be removed from the index. Any way to prevent this?
(In reply to comment #34) Ian, Thanks for trying out the patch and debugging the problem. The problem seems to be different. It is likely possible that the indexer is confused with the build path. The same component which is a source folder seems to be added as a class folder. Can you please verify the build path.
(In reply to comment #35) > (In reply to comment #34) > Ian, Thanks for trying out the patch and debugging the problem. The problem > seems to be different. Yes the problem is definitely a new one. Your patch has definitely worked as expected. Where as before our index file was not even found, now it is. >It is likely possible that the indexer is confused with > the build path. The same component which is a source folder seems to be added > as a class folder. Can you please verify the build path. What do you mean by "check the build path"? Like checking the eclipse preferences, or something while debugging?
(In reply to comment #36) > Yes the problem is definitely a new one. Your patch has definitely worked as > expected. Where as before our index file was not even found, now it is. Could I go ahead and release the patch? > What do you mean by "check the build path"? Like checking the eclipse > preferences, or something while debugging? I meant the project build path(.classpath file).
(In reply to comment #37) > (In reply to comment #36) > > Yes the problem is definitely a new one. Your patch has definitely worked as > > expected. Where as before our index file was not even found, now it is. > Could I go ahead and release the patch? > Yes, Satyam, go ahead :-)
Released the patch on 3.6.1
(In reply to comment #36) > (In reply to comment #35) > > (In reply to comment #34) > > Ian, Thanks for trying out the patch and debugging the problem. The problem > > seems to be different. > > Yes the problem is definitely a new one. Your patch has definitely worked as > expected. Where as before our index file was not even found, now it is. Frederic, Satyam what follow up is needed for the new issue ? (In reply to comment #39) > Released the patch on 3.6.1 Satyam, shouldn't this also be on HEAD ?
(In reply to comment #36) > (In reply to comment #35) > > (In reply to comment #34) > > Ian, Thanks for trying out the patch and debugging the problem. The problem > > seems to be different. > > Yes the problem is definitely a new one. Your patch has definitely worked as > expected. Where as before our index file was not even found, now it is. > Ian, please open a new bug for this new issue, thanks
(In reply to comment #40) > (In reply to comment #39) > > Released the patch on 3.6.1 > > Satyam, shouldn't this also be on HEAD ? As per comment 15, we might introduce an new API to make this fix better. We're waiting for the come back of Jerome who designed the SearchParticipant API initially to verify whether it's accurate or no... So, we'll continue to work on this for 3.7 soon, but as we do not know whether it will be before or after 3.6.1 delivery. Hence, we preferred to set this bug as resolved for 3.6.1 and reopen it when the work on this for 3.7 will restart.
I talked with Jerome about this and we agreed that a new API is not really necessary. The proposed fix for 3.6.1 addresses the client needs without any change in his code and seems definitely acceptable even for 3.7. Furthermore, not adding a new API will make the fix identical for both streams which it simpler and also sounds better to verify that it has no undesirable side effects (as it will be used in both versions)... So, from my side, I would vote to release the patch as this in HEAD...
I have opened Bug 323392 to track my issue mentioned in comment 32, comment 33 and comment 34.
(In reply to comment #29) > Created an attachment (id=177073) [details] > Proposed patch for 3.6.1 Patch looks good, +1 for 3.6.1. I think we should release it as it is for 3.7 M2 even as we explore any additional changes that are required in the context of Bug 323392.
Released on HEAD
Verified for 3.6.1 RC2 by code inspection.
Verified for 3.7M2
