329448 – [Help][Search] SearchManager and SearchServlet not re-entrant, causes Nullpointer exception on Infocenter help search (access from another pc in network with rcp application)

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

Bug 329448 - [Help][Search] SearchManager and SearchServlet not re-entrant, causes Nullpointer exception on Infocenter help search (access from another pc in network with rcp application)

Summary: [Help][Search] SearchManager and SearchServlet not re-entrant, causes Nullpoi...

Status:	RESOLVED FIXED

Alias:	None

Product:	Platform
Classification:	Eclipse Project
Component:	User Assistance (show other bugs)
Version:	3.4
Hardware:	PC Windows XP

Importance:	P3 normal with 1 vote (vote)
Target Milestone:	3.7 M4
Assignee:	Chris Goldthorpe
QA Contact:

URL:
Whiteboard:
Keywords:

Depends on:
Blocks:

Reported:	2010-11-04 11:52 EDT by mgeritz
Modified:	2010-11-16 18:39 EST (History)
CC List:	1 user (show)

See Also:

Attachments
JUnit test which illustrates the problem (5.56 KB, patch) 2010-11-12 17:54 EST, Chris Goldthorpe	no flags	Details \| Diff
Patch which makes SearchManager reentrant (5.56 KB, patch) 2010-11-16 13:07 EST, Chris Goldthorpe	no flags	Details \| Diff
Improved Patch (5.56 KB, patch) 2010-11-16 18:36 EST, Chris Goldthorpe	no flags	Details \| Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description mgeritz

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

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

2010-11-12 17:54:44 EST

Created attachment 183055 [details]
JUnit test which illustrates the problem

Comment 3 Chris Goldthorpe

2010-11-16 13:07:28 EST

Created attachment 183250 [details]
Patch which makes SearchManager reentrant

Comment 4 Chris Goldthorpe

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

2010-11-16 18:39:30 EST

Improved Patch committed to HEAD, Fixed