[jira] [Comment Edited] (HTRACE-119) detach of NullScope singleton should not fail

["Colin Patrick McCabe (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HTRACE-119?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14341937#comment-14341937 ] 

Colin Patrick McCabe edited comment on HTRACE-119 at 3/2/15 7:27 PM:
---------------------------------------------------------------------

Hi [~iwasakims],

Thanks for explaining that!  I feel silly now.  You are right that the way htrace is handling {{NullScope}} is incorrect right now.  By handing out the same {{NullScope}} instance to everyone, and only allowing detach once on that singleton, we are making correct user code fail.

I wonder if, rather than changing the code path in {{TraceScope}}, we could just add a {{NullScope#detach}} function that does nothing (and returns null).  It seems like this would fix the problem, while not allowing double detach when tracing was on.  \[edit: actually patch 001 does not allow double detach in the non-NullScope case... see below\]  There is still an issue with us not catching double detach when {{NullScope}} is in use, but perhaps the optimization of avoiding creating a new scope when tracing is off outweighs that concern?

Take a look at HTRACE-119-002.patch...


was (Author: cmccabe):
Hi [~iwasakims],

Thanks for explaining that!  I feel silly now.  You are right that the way htrace is handling {{NullScope}} is incorrect right now.  By handing out the same {{NullScope}} instance to everyone, and only allowing detach once on that singleton, we are making correct user code fail.

I wonder if, rather than changing the code path in {{TraceScope}}, we could just add a {{NullScope#detach}} function that does nothing (and returns null).  It seems like this would fix the problem, while not allowing double detach when tracing was on.  There is still an issue with us not catching double detach when {{NullScope}} is in use, but perhaps the optimization of avoiding creating a new scope when tracing is off outweighs that concern?

Take a look at HTRACE-119-002.patch...

> detach of NullScope singleton should not fail
> ---------------------------------------------
>
>                 Key: HTRACE-119
>                 URL: https://issues.apache.org/jira/browse/HTRACE-119
>             Project: HTrace
>          Issue Type: Bug
>    Affects Versions: master
>            Reporter: Masatake Iwasaki
>            Assignee: Masatake Iwasaki
>         Attachments: HTRACE-119.001.patch, HTRACE-119.002.patch, HTRACE-119.003.patch
>
>
> The {{detach}} method of NullScope singleton is possible to be called multiple times by users avoiding conditionals by the assumption that NullScope#close and NullScope#detach results in no-op. Calling {{detach}} of NullScope singleton multiple times throws RuntimeException by HTRACE-101 now.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)