Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Correctly close resources in BitmapUtils. #440

Merged
merged 2 commits into from
Oct 19, 2022
Merged

Correctly close resources in BitmapUtils. #440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f9e4161
Correctly close resources in BitmapUtils.
vanniktech Sep 26, 2022
8c5a11a
Code Review.
vanniktech Sep 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Added options to customize colors of toolbar back button, title and menu texts. [#437](https://github.com/CanHub/Android-Image-Cropper/issues/437)
### Fixed
- Fixed and issue where setting toolbar color to white would do nothing. [#437](https://github.com/CanHub/Android-Image-Cropper/issues/437)
- Correctly close resources in BitmapUtils. [#440](https://github.com/CanHub/Android-Image-Cropper/issues/440)

## [4.3.2] - 08/09/2022
### Fixed
Expand Down
2 changes: 1 addition & 1 deletion cropper/src/main/java/com/canhub/cropper/BitmapCroppingWorkerJob.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class BitmapCroppingWorkerJob(
this.sampleSize = sampleSize
}

constructor(uri: Uri?, sampleSize: Int) {
constructor(uri: Uri, sampleSize: Int) {
bitmap = null
this.uri = uri
error = null
Expand Down
82 changes: 30 additions & 52 deletions cropper/src/main/java/com/canhub/cropper/BitmapUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,9 @@ import androidx.exifinterface.media.ExifInterface
import com.canhub.cropper.CropImageView.RequestSizeOptions
import com.canhub.cropper.common.CommonVersionCheck.isAtLeastQ29
import com.canhub.cropper.utils.getUriForFile
import java.io.Closeable
import java.io.File
import java.io.FileNotFoundException
import java.io.IOException
import java.io.InputStream
import java.io.OutputStream
import java.lang.ref.WeakReference
import javax.microedition.khronos.egl.EGL10
import javax.microedition.khronos.egl.EGLConfig
Expand Down Expand Up @@ -426,23 +423,19 @@ internal object BitmapUtils {
compressFormat: CompressFormat,
compressQuality: Int,
customOutputUri: Uri?,
): Uri? {
): Uri {
val newUri = customOutputUri ?: buildUri(context, compressFormat)
var outputStream: OutputStream? = null
try {
outputStream = context.contentResolver.openOutputStream(newUri!!, WRITE_AND_TRUNCATE)

bitmap.compress(compressFormat, compressQuality, outputStream)
} finally {
closeSafe(outputStream)
return context.contentResolver.openOutputStream(newUri, WRITE_AND_TRUNCATE).use {
bitmap.compress(compressFormat, compressQuality, it)
newUri
}
return newUri
}

private fun buildUri(
context: Context,
compressFormat: CompressFormat
): Uri? =
): Uri =
try {
val ext = when (compressFormat) {
CompressFormat.JPEG -> ".jpg"
Expand Down Expand Up @@ -668,16 +661,12 @@ internal object BitmapUtils {
*/
@Throws(FileNotFoundException::class)
private fun decodeImageForOption(resolver: ContentResolver, uri: Uri): BitmapFactory.Options {
var stream: InputStream? = null
return try {
stream = resolver.openInputStream(uri)
return resolver.openInputStream(uri).use {
val options = BitmapFactory.Options()
options.inJustDecodeBounds = true
BitmapFactory.decodeStream(stream, EMPTY_RECT, options)
BitmapFactory.decodeStream(it, EMPTY_RECT, options)
options.inJustDecodeBounds = false
options
} finally {
closeSafe(stream)
}
}

Expand All @@ -692,14 +681,12 @@ internal object BitmapUtils {
options: BitmapFactory.Options
): Bitmap? {
do {
var stream: InputStream? = null
try {
stream = resolver.openInputStream(uri)
return BitmapFactory.decodeStream(stream, EMPTY_RECT, options)
} catch (e: OutOfMemoryError) {
options.inSampleSize *= 2
} finally {
closeSafe(stream)
resolver.openInputStream(uri).use {
try {
return BitmapFactory.decodeStream(it, EMPTY_RECT, options)
} catch (e: OutOfMemoryError) {
options.inSampleSize *= 2
}
}
} while (options.inSampleSize <= 512)
throw CropException.FailedToDecodeImage(uri)
Expand Down Expand Up @@ -727,21 +714,25 @@ internal object BitmapUtils {
rect.width(), rect.height(), reqWidth, reqHeight
)
)
val stream = context.contentResolver.openInputStream(uri)
val decoder = BitmapRegionDecoder.newInstance(stream!!, false)
do {

context.contentResolver.openInputStream(uri).use {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this stream closed automatically? I could not find something that guarantee it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you looked at the implementation of use?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know the use of the keyword use either. I just looked it up. This is the given description of use in kotlin.

Executes the given block function on this resource and then closes it down correctly whether an exception is thrown or not.

Links for more info

  1. Kotlin use documentation
  2. A stackoverflow explanation of use

val decoder = BitmapRegionDecoder.newInstance(it!!, false)

try {
return BitmapSampled(
decoder!!.decodeRegion(rect, options),
options.inSampleSize
)
} catch (e: OutOfMemoryError) {
options.inSampleSize *= 2
do {
try {
return BitmapSampled(
decoder!!.decodeRegion(rect, options),
options.inSampleSize
)
} catch (e: OutOfMemoryError) {
options.inSampleSize *= 2
}
} while (options.inSampleSize <= 512)

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers.

This expression contains a magic number. Consider defining it to a well named constant.
} finally {
decoder?.recycle()
}
} while (options.inSampleSize <= 512)

closeSafe(stream)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was no try / catch / finally here

decoder?.recycle()
}
} catch (e: Exception) {
throw CropException.FailedToLoadBitmap(uri, e.message)
}
Expand Down Expand Up @@ -934,19 +925,6 @@ internal object BitmapUtils {
}
}

/**
* Close the given closeable object (Stream) in a safe way: check if it is null and catch-log
* exception thrown.
*
* @param closeable the closable object to close
*/
private fun closeSafe(closeable: Closeable?) {
try {
closeable?.close()
} catch (ignored: IOException) {
}
}

/**
* Holds bitmap instance and the sample size that the bitmap was loaded/cropped with.
*/
Expand Down