Re: [cp-patches] FYI: Stability fix for gnu/CORBA/SocketRepository

[Mark Wielaard]

Hi Audrius,

On Mon, 2005-10-31 at 12:20 +0100, Meskauskas Audrius wrote:
> This patch fixes some hanging problems that I observed when debugging
> my  CORBA game example. These problems stayed unnoticed during tests
> because  the game normally lasts much longer that it takes the test to
> complete.

I am looking forward to the game!

> 2005-10-31  Audrius Meskauskas  <address@hidden>
> 
> * gnu/CORBA/SocketRepository.java (not_reusable, gc): New methods.
> (sockets): Use hashtable.
> [...]
>    /**
>     * The socket map.
>     */
> -  private static HashMap sockets = new HashMap();
> -
> +  private static Hashtable sockets = new Hashtable();
> +  
>    /**
> -   * Put a socket.
> +   * Put a socket. This method also discards all not reusable sockets
> from
> +   * the map.
>     *
>     * @param key as socket key.
>     *
> @@ -67,6 +69,36 @@
>    public static void put_socket(Object key, Socket s)
>    {
>      sockets.put(key, s);
> +    gc();
> +  }
> +  
> +  /**
> +   * Removes all non reusable sockets.
> +   */
> +  public static void gc()
> +  {
> +    Iterator iter = sockets.entrySet().iterator();
> +    
> +    Map.Entry e;
> +    Socket sx;
> +    
> +    while (iter.hasNext())
> +      {
> +        e = (Map.Entry) iter.next();
> +        sx = (Socket) e.getValue();
> +        
> +        if (not_reusable(sx))
> +          iter.remove();
> +      }
> +  }

This doesn't seem thread-safe. If two threads call put_socket() (or any
other call manipulating the sockets Hashtable) then the Iterator will
not behave correctly (it will throw ConcurrentModificationException).
All uses of sockets should be wrapped inside a synchronized(sockets) { }
block unless you can proof that sockets cannot be manipulated by
multiple threads at the same time. (Hashtable is only thread-safe for
single operations, but not for multiple operations that should be
treated as one change to the collection.)

Cheers,

Mark

signature.asc
Description: This is a digitally signed message part