Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 329448

Summary: [Help][Search] SearchManager and SearchServlet not re-entrant, causes Nullpointer exception on Infocenter help search (access from another pc in network with rcp application)
Product: [Eclipse Project] Platform Reporter: mgeritz
Component: User AssistanceAssignee: Chris Goldthorpe <cgold>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: cgold
Version: 3.4   
Target Milestone: 3.7 M4   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
JUnit test which illustrates the problem
none
Patch which makes SearchManager reentrant
none
Improved Patch none

Description mgeritz CLA 2010-11-04 11:52:11 EDT 
Build Identifier: I20080617-2000

Searching from my local machine with locally hostet infocenter on tomcat works.
When I try to search from an rcp application running on another computer I get the following error:

Während "Lokale Hilfe" ist ein interner Fehler aufgetreten.
java.lang.NullPointerException
 at org.eclipse.help.internal.util.URLCoder.encode(URLCoder.java:20)
 at org.eclipse.help.internal.search.SearchResults.addHits(SearchResults.java:51)
 at org.eclipse.help.internal.search.SearchManager$BufferedSearchHitCollector.flush(SearchManager.java:183)
 at org.eclipse.help.internal.search.SearchManager.searchLocalAndRemote(SearchManager.java:122)
 at org.eclipse.help.internal.search.SearchManager.search(SearchManager.java:78)
 at org.eclipse.help.internal.search.federated.LocalHelp.run(LocalHelp.java:54)
 at org.eclipse.help.internal.search.federated.FederatedSearchJob.run(FederatedSearchJob.java:38)
 at org.eclipse.core.internal.jobs.Worker.run(Worker.java:55)

The problem is on the client side.


Reproducible: Always

Steps to Reproduce:
1. Connect to remote Infocenter hosted on tomcat (not the same machine)
2. Use the search function
Comment 1 Chris Goldthorpe CLA 2010-11-12 17:53:49 EST 
I think I know what is happening. SearchManager has a local variable "wordsSearched" which is set when the search is started and reset to null at the end of the search. As a result a single instance of SearchManager is not thread safe and it is not safe to reuse it.

The help system always uses the same instance of SearchManager - for example BaseHelpSystem.getSearchManager().search(query, collector, new NullProgressMonitor()); If two different threads called SearchManager.search() they could see the exception which you saw.

There are a couple of options for solving the problem - either make the SearchManager class thread safe or create a new SearchManager object for every search. I'm not sure why this problem has not been reported before but it needs to be fixed.
Comment 2 Chris Goldthorpe CLA 2010-11-12 17:54:44 EST 
Created attachment 183055 [details]
JUnit test which illustrates the problem
Comment 3 Chris Goldthorpe CLA 2010-11-16 13:07:28 EST 
Created attachment 183250 [details]
Patch which makes SearchManager reentrant
Comment 4 Chris Goldthorpe CLA 2010-11-16 18:36:21 EST 
Created attachment 183270 [details]
Improved Patch

I added a test for accessing the SearchServlet in parallel and that showed up a bug in SearchServlet where it was using instance variables. This patch includes the fixes and new tests for the SearchServlet and the SearchManager.
Comment 5 Chris Goldthorpe CLA 2010-11-16 18:39:30 EST 
Improved Patch committed to HEAD, Fixed