[10] Review Request: 8183576 Synchronization in BufferedImage.setRGB(int x, int y, int rgb) is not necessary

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[10] Review Request: 8183576 Synchronization in BufferedImage.setRGB(int x, int y, int rgb) is not necessary

Sergey Bylokhov
Hello,
Please review the fix for jdk10.

Bug: https://bugs.openjdk.java.net/browse/JDK-8183576
Webrev can be found at: http://cr.openjdk.java.net/~serb/8183576/webrev.00

The method setRGB(int x, int y, int rgb) in BufferedImage is synchronized, but all other methods in this class are not. For example the similar method:
  setRGB(int startX, int startY, int w, int h, int[] rgbArray, int offset, int scansize)
and get methods:
  getRGB(int x, int y)
  getRGB(int, int, int, int, int[], int, int).

Since the BufferedImage class is not thread-safe it is unclear why the only this method was marked as synchronized.

(Other possible fix: mark this class as a thread-safe and synchronize access to internal data in other methods).

CSR will created after technical review.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] Review Request: 8183576 Synchronization in BufferedImage.setRGB(int x, int y, int rgb) is not necessary

Philip Race
This seems anomalous to me too. I don't see any likely real problem with
removing it ..

-phil.

On 07/05/2017 10:31 AM, Sergey Bylokhov wrote:

> Hello,
> Please review the fix for jdk10.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8183576
> Webrev can be found at: http://cr.openjdk.java.net/~serb/8183576/webrev.00
>
> The method setRGB(int x, int y, int rgb) in BufferedImage is synchronized, but all other methods in this class are not. For example the similar method:
>    setRGB(int startX, int startY, int w, int h, int[] rgbArray, int offset, int scansize)
> and get methods:
>    getRGB(int x, int y)
>    getRGB(int, int, int, int, int[], int, int).
>
> Since the BufferedImage class is not thread-safe it is unclear why the only this method was marked as synchronized.
>
> (Other possible fix: mark this class as a thread-safe and synchronize access to internal data in other methods).
>
> CSR will created after technical review.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] Review Request: 8183576 Synchronization in BufferedImage.setRGB(int x, int y, int rgb) is not necessary

Jim Graham-5
In reply to this post by Sergey Bylokhov
Looks fine to me...

                        ...jim

On 7/5/17 10:31 AM, Sergey Bylokhov wrote:

> Hello,
> Please review the fix for jdk10.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8183576
> Webrev can be found at: http://cr.openjdk.java.net/~serb/8183576/webrev.00
>
> The method setRGB(int x, int y, int rgb) in BufferedImage is synchronized, but all other methods in this class are not. For example the similar method:
>    setRGB(int startX, int startY, int w, int h, int[] rgbArray, int offset, int scansize)
> and get methods:
>    getRGB(int x, int y)
>    getRGB(int, int, int, int, int[], int, int).
>
> Since the BufferedImage class is not thread-safe it is unclear why the only this method was marked as synchronized.
>
> (Other possible fix: mark this class as a thread-safe and synchronize access to internal data in other methods).
>
> CSR will created after technical review.
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] Review Request: 8183576 Synchronization in BufferedImage.setRGB(int x, int y, int rgb) is not necessary

Prahalad kumar Narayanan
In reply to this post by Sergey Bylokhov
Hello Sergey

I don’t see any trouble in removing 'synchronized' from the method signature.
The change looks good.

Thank you
Have a good day

Prahalad N.


-----Original Message-----
From: Sergey Bylokhov
Sent: Wednesday, July 05, 2017 11:01 PM
To: 2d-Dev
Cc: Jim A Graham
Subject: [OpenJDK 2D-Dev] [10] Review Request: 8183576 Synchronization in BufferedImage.setRGB(int x, int y, int rgb) is not necessary

Hello,
Please review the fix for jdk10.

Bug: https://bugs.openjdk.java.net/browse/JDK-8183576
Webrev can be found at: http://cr.openjdk.java.net/~serb/8183576/webrev.00

The method setRGB(int x, int y, int rgb) in BufferedImage is synchronized, but all other methods in this class are not. For example the similar method:
  setRGB(int startX, int startY, int w, int h, int[] rgbArray, int offset, int scansize) and get methods:
  getRGB(int x, int y)
  getRGB(int, int, int, int, int[], int, int).

Since the BufferedImage class is not thread-safe it is unclear why the only this method was marked as synchronized.

(Other possible fix: mark this class as a thread-safe and synchronize access to internal data in other methods).

CSR will created after technical review.
Loading...