harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim Ellison <t.p.elli...@gmail.com>
Subject Re: svn commit: r824006 - in /harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/net: DatagramPacket.java DatagramSocket.java
Date Mon, 12 Oct 2009 11:47:29 GMT
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$
>                  }
> 
> 
> 

Mime
View raw message