largetext icon indicating copy to clipboard operation
largetext copied to clipboard

ThreadSafeLargeText is not thread safe

Open Groostav opened this issue 9 years ago • 1 comments

The ThreadSafeLargeTexts use of a static ThreadLocal makes it not thread safe.


    //written in kotlin, corresponding java can be provided if need be.
    @Test fun `when loading text on multiple threads should not clobber`(){

        val executor = Executors.newFixedThreadPool(2)

        fun loadFile(path: String): CharSequence{
            return LargeTextFactory.newBuilder().build().loadThreadSafe(Paths.get(path))
        }

        val job1 = executor.submit(Callable { loadFile("C:/temp/input1") })
        val job2 = executor.submit(Callable { loadFile("C:/temp/input2") })

        val output1 = job1.get()
        val output2 = job2.get()

        assertThat(output1[0]).isNotEqualTo(output2[0]) //this assertion fails but it should pass.
        //note that interestingly, they both toString with the correct contents, 
        //but calling output.charAt(0) will give a different value than output.toString().charAt(0) for one of the two largetexts.
    }

where we have the files C:/temp/input1 containing abc and the file C:/temp/input2 containing xyz

Groostav avatar Jan 17 '17 23:01 Groostav

OK, sorry for the late answer, and I don't even know whether you care anymore -- anyway, I think there is a confusion here as to what I meant by @ThreadSafe. So, let me explain...

A @ThreadSafe instance of a LargeText object means that you can use this same LargeText instance from multiple threads without worry of any thread impeding on the other, and the way it does that is by making the crux of the decoding process, which is an instance variable, a ThreadLocal instance.

But what you do here is different: you create two instances from the same thread, as such the ThreadLocal instance is shared since this is the same thread!

This is therefore a real bug.

I have not been working with this project for some time but if you are still interested, well, I can go and fix that code.

fge avatar Dec 04 '23 20:12 fge