You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Shai Erera (JIRA)" <ji...@apache.org> on 2009/08/13 15:00:16 UTC

[jira] Created: (LUCENE-1805) CloseableThreadLocal should allow null Objects

CloseableThreadLocal should allow null Objects
----------------------------------------------

                 Key: LUCENE-1805
                 URL: https://issues.apache.org/jira/browse/LUCENE-1805
             Project: Lucene - Java
          Issue Type: Bug
          Components: contrib/analyzers
    Affects Versions: 2.4.1
            Reporter: Shai Erera
             Fix For: 2.9


CloseableThreadLocal does not allow null Objects in its get() method, but does nothing to prevent them in set(Object). The comment in get() before assert v != null is irrelevant - the application might have passed null.

Null is an important value for Analyzers. Since tokenStreams (a ThreadLocal private member in Analyzer) is not accessible by extending classes, the only way for an Analyzer to reset the tokenStreams is by calling setPreviousTokenStream(null).

I will post a patch w/ a test

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org


[jira] Commented: (LUCENE-1805) CloseableThreadLocal should allow null Objects

Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LUCENE-1805?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12742836#action_12742836 ] 

Michael McCandless commented on LUCENE-1805:
--------------------------------------------

bq. Don't you want to move the test to o.a.l.util as well?

Woops, right, I'll do that before committing.

> CloseableThreadLocal should allow null Objects
> ----------------------------------------------
>
>                 Key: LUCENE-1805
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1805
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: contrib/analyzers
>    Affects Versions: 2.4.1
>            Reporter: Shai Erera
>            Assignee: Michael McCandless
>             Fix For: 2.9
>
>         Attachments: LUCENE-1805.patch, LUCENE-1805.patch
>
>
> CloseableThreadLocal does not allow null Objects in its get() method, but does nothing to prevent them in set(Object). The comment in get() before assert v != null is irrelevant - the application might have passed null.
> Null is an important value for Analyzers. Since tokenStreams (a ThreadLocal private member in Analyzer) is not accessible by extending classes, the only way for an Analyzer to reset the tokenStreams is by calling setPreviousTokenStream(null).
> I will post a patch w/ a test

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org


[jira] Commented: (LUCENE-1805) CloseableThreadLocal should allow null Objects

Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LUCENE-1805?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12742835#action_12742835 ] 

Shai Erera commented on LUCENE-1805:
------------------------------------

I was just commenting that I should remove the misleading comment :).

It looks good. Don't you want to move the test to o.a.l.util as well?

> CloseableThreadLocal should allow null Objects
> ----------------------------------------------
>
>                 Key: LUCENE-1805
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1805
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: contrib/analyzers
>    Affects Versions: 2.4.1
>            Reporter: Shai Erera
>            Assignee: Michael McCandless
>             Fix For: 2.9
>
>         Attachments: LUCENE-1805.patch, LUCENE-1805.patch
>
>
> CloseableThreadLocal does not allow null Objects in its get() method, but does nothing to prevent them in set(Object). The comment in get() before assert v != null is irrelevant - the application might have passed null.
> Null is an important value for Analyzers. Since tokenStreams (a ThreadLocal private member in Analyzer) is not accessible by extending classes, the only way for an Analyzer to reset the tokenStreams is by calling setPreviousTokenStream(null).
> I will post a patch w/ a test

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org


[jira] Commented: (LUCENE-1805) CloseableThreadLocal should allow null Objects

Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LUCENE-1805?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12742831#action_12742831 ] 

Shai Erera commented on LUCENE-1805:
------------------------------------

Oops, you're right. I overlooked it. So maybe the second test case I added is not that important, even though it wouldn't hurt to keep it.

> CloseableThreadLocal should allow null Objects
> ----------------------------------------------
>
>                 Key: LUCENE-1805
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1805
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: contrib/analyzers
>    Affects Versions: 2.4.1
>            Reporter: Shai Erera
>            Assignee: Michael McCandless
>             Fix For: 2.9
>
>         Attachments: LUCENE-1805.patch
>
>
> CloseableThreadLocal does not allow null Objects in its get() method, but does nothing to prevent them in set(Object). The comment in get() before assert v != null is irrelevant - the application might have passed null.
> Null is an important value for Analyzers. Since tokenStreams (a ThreadLocal private member in Analyzer) is not accessible by extending classes, the only way for an Analyzer to reset the tokenStreams is by calling setPreviousTokenStream(null).
> I will post a patch w/ a test

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org


[jira] Updated: (LUCENE-1805) CloseableThreadLocal should allow null Objects

Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/LUCENE-1805?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Shai Erera updated LUCENE-1805:
-------------------------------

    Attachment: LUCENE-1805.patch

Remove assert v != null and added a test case.

Funny, but it's as if CTL itself could cause this bug. If you call ctl.get() w/o setting anything first, it creates a WeakReference and set the initValue(), which is ... null !. The first call to get() will return null, the second would fail. :)

BTW, TestCloseableThreadLocal is under o.a.l.index. Shouldn't it be under o.a.l.util? Can whoever assigns this to himself move it?

> CloseableThreadLocal should allow null Objects
> ----------------------------------------------
>
>                 Key: LUCENE-1805
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1805
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: contrib/analyzers
>    Affects Versions: 2.4.1
>            Reporter: Shai Erera
>             Fix For: 2.9
>
>         Attachments: LUCENE-1805.patch
>
>
> CloseableThreadLocal does not allow null Objects in its get() method, but does nothing to prevent them in set(Object). The comment in get() before assert v != null is irrelevant - the application might have passed null.
> Null is an important value for Analyzers. Since tokenStreams (a ThreadLocal private member in Analyzer) is not accessible by extending classes, the only way for an Analyzer to reset the tokenStreams is by calling setPreviousTokenStream(null).
> I will post a patch w/ a test

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org


[jira] Assigned: (LUCENE-1805) CloseableThreadLocal should allow null Objects

Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/LUCENE-1805?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Michael McCandless reassigned LUCENE-1805:
------------------------------------------

    Assignee: Michael McCandless

> CloseableThreadLocal should allow null Objects
> ----------------------------------------------
>
>                 Key: LUCENE-1805
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1805
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: contrib/analyzers
>    Affects Versions: 2.4.1
>            Reporter: Shai Erera
>            Assignee: Michael McCandless
>             Fix For: 2.9
>
>         Attachments: LUCENE-1805.patch
>
>
> CloseableThreadLocal does not allow null Objects in its get() method, but does nothing to prevent them in set(Object). The comment in get() before assert v != null is irrelevant - the application might have passed null.
> Null is an important value for Analyzers. Since tokenStreams (a ThreadLocal private member in Analyzer) is not accessible by extending classes, the only way for an Analyzer to reset the tokenStreams is by calling setPreviousTokenStream(null).
> I will post a patch w/ a test

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org


[jira] Updated: (LUCENE-1805) CloseableThreadLocal should allow null Objects

Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/LUCENE-1805?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Michael McCandless updated LUCENE-1805:
---------------------------------------

    Attachment: LUCENE-1805.patch

Attached small tweaks to the test (removed the misleading comment).

I think it's ready to commit.  If it looks ok Shai I'll commit shortly...

> CloseableThreadLocal should allow null Objects
> ----------------------------------------------
>
>                 Key: LUCENE-1805
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1805
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: contrib/analyzers
>    Affects Versions: 2.4.1
>            Reporter: Shai Erera
>            Assignee: Michael McCandless
>             Fix For: 2.9
>
>         Attachments: LUCENE-1805.patch, LUCENE-1805.patch
>
>
> CloseableThreadLocal does not allow null Objects in its get() method, but does nothing to prevent them in set(Object). The comment in get() before assert v != null is irrelevant - the application might have passed null.
> Null is an important value for Analyzers. Since tokenStreams (a ThreadLocal private member in Analyzer) is not accessible by extending classes, the only way for an Analyzer to reset the tokenStreams is by calling setPreviousTokenStream(null).
> I will post a patch w/ a test

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org


[jira] Resolved: (LUCENE-1805) CloseableThreadLocal should allow null Objects

Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/LUCENE-1805?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Michael McCandless resolved LUCENE-1805.
----------------------------------------

    Resolution: Fixed

Thanks Shai!

> CloseableThreadLocal should allow null Objects
> ----------------------------------------------
>
>                 Key: LUCENE-1805
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1805
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: contrib/analyzers
>    Affects Versions: 2.4.1
>            Reporter: Shai Erera
>            Assignee: Michael McCandless
>             Fix For: 2.9
>
>         Attachments: LUCENE-1805.patch, LUCENE-1805.patch
>
>
> CloseableThreadLocal does not allow null Objects in its get() method, but does nothing to prevent them in set(Object). The comment in get() before assert v != null is irrelevant - the application might have passed null.
> Null is an important value for Analyzers. Since tokenStreams (a ThreadLocal private member in Analyzer) is not accessible by extending classes, the only way for an Analyzer to reset the tokenStreams is by calling setPreviousTokenStream(null).
> I will post a patch w/ a test

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org


[jira] Commented: (LUCENE-1805) CloseableThreadLocal should allow null Objects

Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LUCENE-1805?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12742830#action_12742830 ] 

Michael McCandless commented on LUCENE-1805:
--------------------------------------------

bq.  If you call ctl.get() w/o setting anything first, it creates a WeakReference and set the initValue(), which is ... null

I don't think that's right?

It calls initialValue(), and only if that returns a non-null value will it set it.  Else it doesn't call set?  (So calling get() twice in a row not having called set shouldn't hit an exception).  I think?

> CloseableThreadLocal should allow null Objects
> ----------------------------------------------
>
>                 Key: LUCENE-1805
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1805
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: contrib/analyzers
>    Affects Versions: 2.4.1
>            Reporter: Shai Erera
>             Fix For: 2.9
>
>         Attachments: LUCENE-1805.patch
>
>
> CloseableThreadLocal does not allow null Objects in its get() method, but does nothing to prevent them in set(Object). The comment in get() before assert v != null is irrelevant - the application might have passed null.
> Null is an important value for Analyzers. Since tokenStreams (a ThreadLocal private member in Analyzer) is not accessible by extending classes, the only way for an Analyzer to reset the tokenStreams is by calling setPreviousTokenStream(null).
> I will post a patch w/ a test

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org