[jcifs] Re: Potential concurrency problem

[Ronny Schuetz]

Hi Mike,

thanks for your reply.

> Yup. This is a concurrency error (albeit one that will only occur if
> you have an unresponsive WINS server and rarely at that).
> 
> However, synchronizing switchWINS is not a good solution as it will
> still leave the possibility for concurrency problems.
>
> You should use the NameServiceClient.java lock where it is calling
> switchWINS. Meaning add a synchronized (LOCK) { } around that WINS
> manipulation like:
> 
> 261                 synchronized (LOCK) {
> 262                     if (NbtAddress.isWINS( request.addr ) == false)
> 263                         break;
> 264
> 265                                     /* Message was sent to WINS but
> 266                                      * failed to receive response.
> 267                                      * Try a different WINS server.
> 268                                      */
> 269                     if (request.addr == NbtAddress.getWINSAddress())
> 270                         NbtAddress.switchWINS();
> 271                     request.addr = NbtAddress.getWINSAddress();
> 272                 }

Mhm, I usually tend to synchronize as close as possible to the place
where the variable is changed. Anyway, just found a better solution that
is safe without additional synchronization. Just replace

nbnsIndex = (nbnsIndex + 1) < NBNS.length ? nbnsIndex + 1 : 0;

with

nbnsIndex = NBNS.length > 0 ? (nbnsIndex + 1) % NBNS.length : 0;

This should fix the issue. Worst case, nbnsIndex isn't incremented as
often as switchWINS() has been called, but it cannot exceed the array
bounds. Works fine here.

Best regards,
Ronny