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.

Bug 106989 - Threading model of OptionsPanelController not specified
Summary: Threading model of OptionsPanelController not specified
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Options&Settings (show other bugs)
Version: 6.x
Hardware: All All
: P2 blocker (vote)
Assignee: rmatous
URL:
Keywords: API, THREAD
Depends on:
Blocks: 104298 104299
  Show dependency tree
 
Reported: 2007-06-18 13:54 UTC by Andrei Badea
Modified: 2008-12-22 14:42 UTC (History)
0 users

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
applyChanges, cancel switch into AWT thread (12.94 KB, patch)
2007-07-13 17:57 UTC, rmatous
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrei Badea 2007-06-18 13:54:04 UTC
The Javadoc of OptionsPanelController does not mention the thread in which the class will be called. Every
implementation I have seen assumes that all methods are called in the AWT event thread. This is also the assumption of
the apisupport Options Panel wizard. However, at least the cancel() and applyChanged() methods are called in a
RequestProcessor thread.
Comment 1 Petr Hejl 2007-06-18 14:46:37 UTC
I've met this problem too. When I invoked the options panel twice in short period the second update() call was invoked
_before_ the first invocation of applyChanged(), thus the wrong (old) data were presented to the user. I traced the
behaviour of the api and found the same reason as described here (applyChanged is executed in separate thread).
Unfortunately this imho can't be solved in controller implementation itself.
Comment 2 rmatous 2007-06-27 14:33:10 UTC
Yes, cancel and applyChanged were replanned into working thread:
OptionsWindowAction.java, revision 1.19
date: 2005/10/17 14:02:38;  author: jjancura;  state: Exp;  lines: +30 -6
66694 [perf] Closing of Options dialog takes longer than it used to

The problem mentioned by phejl seems to be valid problem, the second one that occurs my mind is - that  after disposing
OD one would expect that really all the changes would be applied which doesn't need to be true because there is no
guarantee how long the worker thread will run.
Knowing it I wouldn't mind to call cancel and applyChanged from AWT again- I also didn't notice any performance problem
when switched back to AWT.

Method getLookup is called from worker thread:
CategoryModel.java, revision 1.1
date: 2006/12/07 11:57:55;  author: rmatous;  state: Exp;
refactoring with respect to performance
Comment 3 rmatous 2007-07-13 17:53:32 UTC
Anderi is right that calling these methods from AWT thread is what one would expect here, me too. 

Moreover I don't see any reason why saving should mean perf.problem because first only modified settings should be made
persistent and second  both common NB approaches (Preferences, SystemOption) take care about it  and the data are stored
in postponed asynchronous running worker thread. Moreover switch from worker thread into AWT thread shouldn't cause any
harm from backward compatibility point of view.


Comment 4 rmatous 2007-07-13 17:57:12 UTC
Created attachment 45082 [details]
applyChanges, cancel switch into AWT thread
Comment 5 rmatous 2007-07-16 10:35:19 UTC
Attachment applied, still should be documented, so still keep this issue  open:
/cvs/core/options/test/unit/src/org/netbeans/api/options/OptionsDisplayerOpenTest.java,v  <--  new revision: 1.2;
previous revision: 1.1

/cvs/core/options/test/unit/src/org/netbeans/api/options/RegisteredCategory.java,v  <--  RegisteredCategory.java
new revision: 1.2; previous revision: 1.1

/cvs/core/options/nbproject/project.xml,v  <--  project.xml
new revision: 1.13; previous revision: 1.12

/cvs/core/options/src/org/netbeans/modules/options/OptionsDisplayerImpl.java,v  <--  OptionsDisplayerImpl.java
new revision: 1.3; previous revision: 1.2
Comment 6 rmatous 2007-07-16 10:35:57 UTC
Attachment applied, still should be documented, so still keep this issue  open:
/cvs/core/options/test/unit/src/org/netbeans/api/options/OptionsDisplayerOpenTest.java,v  <--  new revision: 1.2;
previous revision: 1.1

/cvs/core/options/test/unit/src/org/netbeans/api/options/RegisteredCategory.java,v  <--  RegisteredCategory.java
new revision: 1.2; previous revision: 1.1

/cvs/core/options/nbproject/project.xml,v  <--  project.xml
new revision: 1.13; previous revision: 1.12

/cvs/core/options/src/org/netbeans/modules/options/OptionsDisplayerImpl.java,v  <--  OptionsDisplayerImpl.java
new revision: 1.3; previous revision: 1.2
Comment 7 Andrei Badea 2007-07-16 11:23:34 UTC
After this change, does the warning in the Javadoc of applyChanges() and cancel() that they can be called even before
update() still stand? Asking because of issue 104299.
Comment 8 rmatous 2007-07-16 14:25:52 UTC
Following note removed from javadoc: "This method can be called even before update () method is called.", this situation
shouldn't happen:

/cvs/core/options/src/org/netbeans/modules/options/CategoryModel.java,v  <--  CategoryModel.java
new revision: 1.7; previous revision: 1.6

/cvs/core/options/test/unit/src/org/netbeans/api/options/RegisteredCategory.java,v  <--  RegisteredCategory.java
new revision: 1.3; previous revision: 1.2

/cvs/core/options/src/org/netbeans/spi/options/OptionsPanelController.java,v  <--  OptionsPanelController.java
new revision: 1.4; previous revision: 1.3
Comment 9 rmatous 2007-07-16 14:39:44 UTC
Method getLookup - added note into javadoc that nobody should rely that this method is called from AWT thread. If this
method should be guaranteed to be called from AWT thread then there couldn't be guaranteed acceptable performance
because then wouldn't be possible to create composed Lookup lazy.

/cvs/core/options/src/org/netbeans/spi/options/OptionsPanelController.java,v  <--  OptionsPanelController.java
new revision: 1.5; previous revision: 1.4
Comment 10 rmatous 2007-07-16 14:48:36 UTC
The last change in javadoc for method getLookup just describes current state