[BTRACE-9] TLS does not implement ThreadLocalPattern correctly about btrace

[jbachorik]

[author="m_hess", created="Fri, 2 Oct 2009 17:07:41 +0200"]

Sorry about the mess with the curly braces. Somehow the formatting did not work as expected.

[jbachorik]

[author="jbachorik", created="Sat, 10 Oct 2009 18:19:11 +0200"]

The whole problem is in the fact that the expression
@tls =

will get transformed into something like
=
ThreadLocal variable = new ThreadLocal() {
protected Object initialValue()

{
return
}

}

as Michael pointed out and this transformation is completely wrong. This implementation works correctly for immutable objects only.

A correct solution would be to generate the following construct instead:
ThreadLocal variable = new ThreadLocal() {
protected Object initialValue()

{
return ;
}

}

Unfortunately, all those transformations are done on an already compiled BTrace probe and extracting the field initializer from the bytecode is almost impossible using the ASM Core APIs only. In order to be able to generate the required code we would need either to use the ASM Tree API for advanced transformations or to pre-process the source code to wrap the initializer expression properly.

Either way, the proper fix would mean significant changes in the BTrace core and as such it will require careful architecting.

A bit "hackish" fix would be to force the objects used as @tls into being "Cloneable" and rewrite the BTraceRuntime method to clone the initValue if possible. I will attach webrev for this patch shortly.

[jbachorik]

[author="jbachorik", created="Sat, 10 Oct 2009 18:20:07 +0200"]

Webrev for the proposed patch

[jbachorik]

[author="m_hess", created="Mon, 12 Oct 2009 09:32:58 +0200"]

Thanks for taking care of this. Unfortunately I don't know how the process can commence.
Is there some action I need to take, like making a review of your patch?
If so, could you point me in the right direction of needed formalities? I don't konw much about webrev.

[jbachorik]

[author="jbachorik", created="Mon, 12 Oct 2009 09:46:15 +0200"]

The webrev is there more for Sundar (who originally wrote the whole BTrace core) to verify that I am not breaking anything.

As for webrev - you can finde index.html inside the archive which you can open in your web browser and browse the patch in a user friendly way. Also, the webrev contains the "btrace.patch" file which you can "hg import" into your btrace working directory, rebuild and test.

[jbachorik]

[author="m_hess", created="Tue, 13 Oct 2009 14:11:41 +0200"]

I imported the patch into my local repository. It looks good so far, the testcase I created runs smoothly now.

"Hackish" - indeed, but it gets the job done for now. Thanks Jaroslav!

I suggest that you enhance the TLS JavaDoc to tell that the instance that is initially declared and created gets cloned and is never used itself. This might avoid some confusion on the users side, if for whatever reasons values/state get lost during the clone() operation.

I could imagine a situation were someone initializes an instance with some value and annotates it with TLS. If the clone() operation (which BTrace has no influence upon) does not clone the aforementioned value, then this might cause some headaches.

Maybe far fetched / too theoretical, but maybe still worth mentioning.

[jbachorik]

[author="jbachorik", created="Mon, 19 Oct 2009 14:56:23 +0200"]

Patch delivered in http://kenai.com/projects/btrace/sources/hg/revision/214

[jbachorik]

[author="jbachorik", created="Mon, 19 Oct 2009 15:15:11 +0200"]

updated user guide

[jbachorik]

[author="jbachorik", created="Wed, 6 Jan 2010 15:54:41 +0100"]

Please, verify and close this issue

[jbachorik]

[author="m_hess", created="Fri, 8 Jan 2010 16:22:05 +0100"]

I would love to close this, but all operations available to me (see screenshot) do not offer any fitting operation.
Jaroslav, can you have my permissions adjusted?