Menu
▾
▴
Re: [ jEdit-devel ] TokenMarker.java question
|
From: Shlomy R. <sre...@gm...> - 2009-10-03 09:40:26
|
Hi,
It turns out that TokenMarker.markTokens is not thread-safe. It needs
to be synchronized. It has a 'context' field, that is potentially
updated each time "markTokens" is called. If called simultaneously by
multiple threads, each may modify it, causing the other threads to use
an incorrect context.
I just verified that the problem happens very frequently with the
TaskList plugin. TaskList has a dockable which shows the tasks in the
current buffer, and also highlights tasks in the text area. To find
the tasks in the buffer, it uses "markTokens" in the background. So,
when this plugin is in use, I experience all kinds of painting issues
and occasionally the NPE that Dale experienced.
Disabling the TaskList plugin gets rid of all these problems.
However, it is not a TaskList problem, "markTokens" simply needs to be
synchronized. If this causes some performance problem in the core,
then we should have two separate paths to markTokens, one for the core
and one for plugins, and make the plugin's path ensure thread-safety,
e.g. by deferring the task to the AWT thread.
Shlomy
On Sat, Oct 3, 2009 at 3:39 AM, Dale Anson <da...@gr...> wrote:
> Actually, I verified that context.parent is occasionally null, which
> causes the NPE when its 'rules' field is referenced. Thinking about
> this some more, I bet is it's a threading issue. It could be that in
> the first check (in the code you sent) it is not null, but is reset to
> null before the method is actually invoked. That would mean my patch
> is not very good since it doesn't address thread-safety.
>
> I did double check, I've got source for jEdit at revision 16233. The
> only change I have is the one I made to this class. I verified that
> I'm using the standard java mode from svn also, and it's while editing
> java files that I know I've seen the problem. I'm not sure if I've
> seen this issue in other modes.
>
> Dale
>
>
> On Fri, Oct 2, 2009 at 2:46 AM, Shlomy Reinstein <sre...@gm...> wrote:
>> I took a more thorough look at the code. It is absolutely IMPOSSIBLE
>> that context.parent is null in the method that generates the NPE for
>> you, unless you've changed something in the jedit core. There is only
>> one place where the method generating the NPE is called:
>> //{{{ check for end of delegate
>> if (context.parent != null
>> && context.parent.inRule != null
>> && checkDelegateEnd(context.parent.inRule))
>> {
>> seenWhitespaceEnd = true;
>> continue main_loop;
>> } //}}}
>>
>> Note the check for "context.parent" not being null before calling this method.
>> An NPE can still be caused by this line, but it won't be for
>> "context.parent" being null, rather it would be for
>> "context.parent.rules" being null.
>>
>> Shlomy
>>
>> On Fri, Oct 2, 2009 at 8:15 AM, Dale Anson <da...@gr...> wrote:
>>> No, I can't reproduce it reliably. Somehow, I get into this state
>>> after a couple of hours of running jEdit. Usually, it happens when
>>> I'm editing somewhere in the middle of a file, I'll hit Enter at the
>>> end of a line to start the next line and then there is the trouble. I
>>> have verified that it is definitely the case that context.parent is
>>> sometimes null. I see in LineContext itself that there are a several
>>> checks for a null parent, but not elsewhere. I think I've only seen
>>> this with java files, but I'm not absolutely certain.
>>>
>>> Do you know of a better thing to do if context.parent is null?
>>>
>>> Dale
>>>
>>>
>>> On Thu, Oct 1, 2009 at 11:31 PM, Shlomy Reinstein <sre...@gm...> wrote:
>>>> Hi,
>>>> Can you reproduce this every time with some example?
>>>> If not , does this happen when you are editing (i.e. pressing enter
>>>> on) the first or second line, or also when editing subsequent lines
>>>> down the file?
>>>> The 'parent' is usually the LineContext of the previous line.
>>>> Shlomy
>>>>
>>>> On Fri, Oct 2, 2009 at 3:33 AM, Dale Anson <da...@gr...> wrote:
>>>>> I entered a patch to fix this, see tracker 2871624. I did some more
>>>>> investigation, and it turns out that context.parent can be null. The
>>>>> patch checks for this case, and if the parent is null, it just
>>>>> continues to use the same context. So far, this seems to be working
>>>>> fine.
>>>>>
>>>>> Dale
>>>>>
>>>>>
>>>>> On Wed, Sep 30, 2009 at 1:07 PM, Alan Ezust <ala...@gm...> wrote:
>>>>>> I've seen an NPE there from time to time also, but I can't reproduce it.
>>>>>>
>>>>>> On Wed, Sep 30, 2009 at 11:50 AM, Dale Anson <da...@gr...> wrote:
>>>>>>>
>>>>>>> On Wed, Sep 30, 2009 at 9:58 AM, Matthieu Casanova
>>>>>>> <cho...@gm...> wrote:
>>>>>>> > But in that code, the fields are used as parameters, so filling the
>>>>>>> > keyword
>>>>>>> > field at line 294 is used to highlight tokens, removing that will lead
>>>>>>> > to
>>>>>>> > bugs.
>>>>>>> > For the NPE at line 294, do you know if it is because of context being
>>>>>>> > null
>>>>>>> > or context.rules ?
>>>>>>>
>>>>>>> I don't know. I didn't look into it far enough. I supposed, though,
>>>>>>> if I'm the only one seeing this issue, that means I've got something
>>>>>>> wrong somewhere.
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> ------------------------------------------------------------------------------
>>>>> Come build with us! The BlackBerry® Developer Conference in SF, CA
>>>>> is the only developer event you need to attend this year. Jumpstart your
>>>>> developing skills, take BlackBerry mobile applications to market and stay
>>>>> ahead of the curve. Join us from November 9-12, 2009. Register now!
>>>>> http://p.sf.net/sfu/devconf
>>>>> --
>>>>> -----------------------------------------------
>>>>> jEdit Developers' List
>>>>> jEd...@li...
>>>>> https://lists.sourceforge.net/lists/listinfo/jedit-devel
>>>>>
>>>>
>>>
>>
>
|
