[jcifs] WriterThreads are leaking

[Shon Vella]

On Oct 23, 2013, at 12:59 AM, Michael B Allen <ioplex at gmail.com> wrote:

> On Mon, Oct 21, 2013 at 2:34 PM, Shon Vella <svella at idauto.net> wrote:
> 
> So w.write( null, -1, null, 0 ) is being called to reset the
> WriterThread before the WriterThread.run routine has entered the
> "ready" state. That sounds very plausible. And your fix sounds exactly
> correct. All WriterThread operations need to verify that the it has
> first reached the ready state.

Yeah, once the child thread gets into synchronized section everything would work fine without even checking the ready variable - it’s just that you can’t predict when it will actually get there the first time. An alternate fix would be to enter a synchronized section before starting the thread and then waiting for first notification right after call to start() before proceeding - I think that would eliminate the need for the ready variable altogether, but since I didn’t have a lot of time to spend on it I didn’t want to mess with the logic any more than necessary.

> I don't think we need to use volatile though because we synchronize explicitly.

Volatile doesn’t really have anything to do with synchronization, but rather is a hint to the compiler/optimizer/JIT that it can’t optimize away accesses to the variable. I don’t know for sure that it’s needed here, but I’ve been bitten enough times in similar situations I like to play it safe with anything that gets modified like that in a loop.

> I have added your fix to the TODO list and bumped it to toward the
> top. Although updates are very few and far between at this point so I
> cannot make promises about when it will actually be released.

No worries on schedule - will use a private build for now.

> Good job diagnosing the problem and coming up with what looks like the
> right fix. That doesn't happen often with JCIFS. The code can be
> tricky to understand. Nice work.

Glad to give back from time to time.

Shon Vella
Developer
iDENTiTYAUTOMATiON