Mailing-List: contact dev-help@harmony.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@harmony.apache.org
Received-SPF: pass (nike.apache.org: domain of t.p.ellison@gmail.com
 designates 209.85.218.227 as permitted sender)
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=message-id:date:from:user-agent:mime-version:to:subject:references
         :in-reply-to:x-enigmail-version:content-type
         :content-transfer-encoding;
        b=l13sx4izl90uzLU9sI8VmvNVaYBizL56DW6Zz1R82EraamEQeekH02pasxMHpiWpvb
         1MbxXHqISTQSeuVN8OOtKpcDZ541xzNfMioYpUXNe4IGIN2+YGA4Y7zEGX9o3saK64y8
         FJJQzPyBgXoJ/WZ5D9sJFnAjbE8ZgaVIM9p4Y=
Message-ID: <4AD31751.3000704@gmail.com>
Date: Mon, 12 Oct 2009 12:47:29 +0100
From: Tim Ellison <t.p.ellison@gmail.com>
User-Agent: Thunderbird 2.0.0.23 (Windows/20090812)
MIME-Version: 1.0
To: dev@harmony.apache.org
Subject: Re: svn commit: r824006 - in
 /harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/net:
 DatagramPacket.java DatagramSocket.java
References: <20091011031344.389E823888DA@eris.apache.org>
In-Reply-To: <20091011031344.389E823888DA@eris.apache.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

With a simple getting being synchronized like this, I assume it is to
get the memory consistency on that field?

Looking at DatagramPacket, all of the methods are synchronized.
Wouldn't it be better to make these fields volatile, rather than
requiring the setters and getters to acquire the lock as part of
synchronization too?

I realize that some methods that access more than one field will still
need to be synchronized.

Regards,
Tim

On 11/Oct/2009 04:13, ndbeyer@apache.org wrote:
> Author: ndbeyer
> Date: Sun Oct 11 03:13:39 2009
> New Revision: 824006
> 
> URL: http://svn.apache.org/viewvc?rev=824006&view=rev
> Log:
> add some package methods to provide consistent synchronization of access to instance fields
> 
> Modified:
>     harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/net/DatagramPacket.java
>     harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/net/DatagramSocket.java
> 
> Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/net/DatagramPacket.java
> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/net/DatagramPacket.java?rev=824006&r1=824005&r2=824006&view=diff
> ==============================================================================
> --- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/net/DatagramPacket.java (original)
> +++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/net/DatagramPacket.java Sun Oct 11 03:13:39 2009
> @@ -215,6 +215,15 @@
>      }
>  
>      /**
> +     * Gets the current capacity value.
> +     * 
> +     * @return the current capacity value
> +     */
> +    synchronized int getCapacity() {
> +        return capacity;
> +    }
> +
> +    /**
>       * Sets the length of the datagram packet. This length plus the offset must
>       * be lesser than or equal to the buffer size.
>       * 
> @@ -230,6 +239,19 @@
>      }
>  
>      /**
> +     * An alternative to {@link #setLength(int)}, that doesn't reset the {@link #capacity}
> +     * field.
> +     * 
> +     * @param len the length of this datagram packet
> +     */
> +    synchronized void setLengthOnly(int len) {
> +        if (0 > len || offset + len > data.length) {
> +            throw new IllegalArgumentException(Msg.getString("K002f")); //$NON-NLS-1$
> +        }
> +        length = len;
> +    }
> +
> +    /**
>       * Sets the port number of the target host of this datagram packet.
>       * 
>       * @param aPort
> 
> Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/net/DatagramSocket.java
> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/net/DatagramSocket.java?rev=824006&r1=824005&r2=824006&view=diff
> ==============================================================================
> --- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/net/DatagramSocket.java (original)
> +++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/net/DatagramSocket.java Sun Oct 11 03:13:39 2009
> @@ -361,8 +361,8 @@
>                      if (e.getMessage().equals(
>                              "The socket does not support the operation")) { //$NON-NLS-1$
>                          // receive packet to temporary buffer
> -                        tempPack = new DatagramPacket(new byte[pack.capacity],
> -                                pack.capacity);
> +                        tempPack = new DatagramPacket(new byte[pack.getCapacity()],
> +                                pack.getCapacity());
>                          impl.receive(tempPack);
>                          // tempPack's length field is now updated, capacity is unchanged 
>                          // let's extract address & port
> @@ -404,11 +404,11 @@
>                      .getOffset(), tempPack.getLength());
>              // we shouldn't update the pack's capacity field in order to be
>              // compatible with RI
> -            pack.length = tempPack.length;
> +            pack.setLengthOnly(tempPack.getLength());
>              pack.setAddress(tempPack.getAddress());
>              pack.setPort(tempPack.getPort());
>          } else {
> -            pack.setLength(pack.capacity);
> +            pack.setLength(pack.getCapacity());
>              impl.receive(pack);
>              // pack's length field is now updated by native code call;
>              // pack's capacity field is unchanged 
> @@ -442,7 +442,7 @@
>          } else {
>              // not connected so the target address is not allowed to be null
>              if (packAddr == null) {
> -                if (pack.port == -1) {
> +                if (pack.getPort() == -1) {
>                      // KA019 Destination address is null
>                      throw new NullPointerException(Msg.getString("KA019")); //$NON-NLS-1$
>                  }
> 
> 
>