Re: Review Request 43567: Always close Deflater/Inflater streams

2016-02-14 Thread Aurora ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43567/#review119199
---



Master (2b48f22) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Feb. 14, 2016, 9:52 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43567/
> ---
> 
> (Updated Feb. 14, 2016, 9:52 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Closing Deflater/Inflater streams explicitly frees their native memory 
> instantly. Otherwise, the memory will only be freed once the object finalizer 
> runs.
> 
> I got the idea from this article 
> http://www.evanjones.ca/java-native-leak-bug.html. Unfortunately, we cannot 
> use the Java close-with-resource feature, as `TTransport` in our current 
> Thrift version does not implement `Closable`.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
> 6b65c0f6cb60b98ff352c4c5d5fed38d52b4b062 
> 
> Diff: https://reviews.apache.org/r/43567/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 43567: Always close Deflater/Inflater streams

2016-02-14 Thread Stephan Erb 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43567/#review119195
---



@ReviewBot retry

- Stephan Erb


On Feb. 14, 2016, 10:52 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43567/
> ---
> 
> (Updated Feb. 14, 2016, 10:52 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Closing Deflater/Inflater streams explicitly frees their native memory 
> instantly. Otherwise, the memory will only be freed once the object finalizer 
> runs.
> 
> I got the idea from this article 
> http://www.evanjones.ca/java-native-leak-bug.html. Unfortunately, we cannot 
> use the Java close-with-resource feature, as `TTransport` in our current 
> Thrift version does not implement `Closable`.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
> 6b65c0f6cb60b98ff352c4c5d5fed38d52b4b062 
> 
> Diff: https://reviews.apache.org/r/43567/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 43567: Always close Deflater/Inflater streams

2016-02-14 Thread Aurora ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43567/#review119194
---



Master (2b48f22) is red with this patch.
  ./build-support/jenkins/build.sh

+ date
Sun Feb 14 21:51:49 UTC 2016
+ ./gradlew -Pq clean build
:buildSrc:clean UP-TO-DATE
:buildSrc:compileJava UP-TO-DATE
:buildSrc:compileGroovy
:buildSrc:processResources UP-TO-DATE
:buildSrc:classes
:buildSrc:jar
:buildSrc:assemble
:buildSrc:compileTestJava UP-TO-DATE
:buildSrc:compileTestGroovy UP-TO-DATE
:buildSrc:processTestResources UP-TO-DATE
:buildSrc:testClasses UP-TO-DATE
:buildSrc:test UP-TO-DATE
:buildSrc:check UP-TO-DATE
:buildSrc:build

FAILURE: Build failed with an exception.

* What went wrong:
A problem occurred configuring root project 'aurora'.
> Could not open proj class cache for build file 
> '/home/jenkins/jenkins-slave/workspace/AuroraBot/build.gradle' 
> (/home/jenkins/.gradle/caches/2.10/scripts/build_e62v1tfg3vtzz7pxxy2kzgwpr/proj).
   > Timeout waiting to lock proj class cache for build file 
'/home/jenkins/jenkins-slave/workspace/AuroraBot/build.gradle' 
(/home/jenkins/.gradle/caches/2.10/scripts/build_e62v1tfg3vtzz7pxxy2kzgwpr/proj).
 It is currently in use by another Gradle instance.
 Owner PID: unknown
 Our PID: 8305
 Owner Operation: unknown
 Our operation: Initialize cache
 Lock file: 
/home/jenkins/.gradle/caches/2.10/scripts/build_e62v1tfg3vtzz7pxxy2kzgwpr/proj/cache.properties.lock

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

BUILD FAILED

Total time: 1 mins 25.822 secs


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Feb. 14, 2016, 9:52 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43567/
> ---
> 
> (Updated Feb. 14, 2016, 9:52 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Closing Deflater/Inflater streams explicitly frees their native memory 
> instantly. Otherwise, the memory will only be freed once the object finalizer 
> runs.
> 
> I got the idea from this article 
> http://www.evanjones.ca/java-native-leak-bug.html. Unfortunately, we cannot 
> use the Java close-with-resource feature, as `TTransport` in our current 
> Thrift version does not implement `Closable`.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
> 6b65c0f6cb60b98ff352c4c5d5fed38d52b4b062 
> 
> Diff: https://reviews.apache.org/r/43567/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 43567: Always close Deflater/Inflater streams

2016-02-14 Thread Bill Farner 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43567/#review119193
---


Ship it!




Ship It!

- Bill Farner


On Feb. 14, 2016, 1:46 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43567/
> ---
> 
> (Updated Feb. 14, 2016, 1:46 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Closing Deflater/Inflater streams explicitly frees their native memory 
> instantly. Otherwise, the memory will only be freed once the object finalizer 
> runs.
> 
> I got the idea from this article 
> http://www.evanjones.ca/java-native-leak-bug.html. Unfortunately, we cannot 
> use the Java close-with-resource feature, as `TTransport` in our current 
> Thrift version does not implement `Closable`.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
> 6b65c0f6cb60b98ff352c4c5d5fed38d52b4b062 
> 
> Diff: https://reviews.apache.org/r/43567/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 43567: Always close Deflater/Inflater streams

2016-02-14 Thread Stephan Erb 


> On Feb. 14, 2016, 9:37 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java, line 164
> > 
> >
> > Note: this `close` cannot be moved to `finally` and cannot be replaced 
> > by a `flush`. It is mandatory at this position.
> 
> Bill Farner wrote:
> I presume this is because closing has a flush effect that renders 
> `outBytes` complete?  A one-line comment to that effect would go a long way.
> 
> However, can you additionally do the `close()` in a finally without 
> causing harm? (and omit from the `catch` branch)

Good call. Done.


- Stephan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43567/#review119187
---


On Feb. 14, 2016, 10:46 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43567/
> ---
> 
> (Updated Feb. 14, 2016, 10:46 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Closing Deflater/Inflater streams explicitly frees their native memory 
> instantly. Otherwise, the memory will only be freed once the object finalizer 
> runs.
> 
> I got the idea from this article 
> http://www.evanjones.ca/java-native-leak-bug.html. Unfortunately, we cannot 
> use the Java close-with-resource feature, as `TTransport` in our current 
> Thrift version does not implement `Closable`.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
> 6b65c0f6cb60b98ff352c4c5d5fed38d52b4b062 
> 
> Diff: https://reviews.apache.org/r/43567/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 43567: Always close Deflater/Inflater streams

2016-02-14 Thread Stephan Erb 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43567/
---

(Updated Feb. 14, 2016, 10:46 p.m.)


Review request for Aurora, John Sirois and Zameer Manji.


Changes
---

Clarifications proposed by Bill.


Repository: aurora


Description
---

Closing Deflater/Inflater streams explicitly frees their native memory 
instantly. Otherwise, the memory will only be freed once the object finalizer 
runs.

I got the idea from this article 
http://www.evanjones.ca/java-native-leak-bug.html. Unfortunately, we cannot use 
the Java close-with-resource feature, as `TTransport` in our current Thrift 
version does not implement `Closable`.


Diffs (updated)
-

  src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
6b65c0f6cb60b98ff352c4c5d5fed38d52b4b062 

Diff: https://reviews.apache.org/r/43567/diff/


Testing
---

./gradlew -Pq build


Thanks,

Stephan Erb

Re: Review Request 43567: Always close Deflater/Inflater streams

2016-02-14 Thread Stephan Erb 


> On Feb. 14, 2016, 9:59 p.m., Aurora ReviewBot wrote:
> > Master (2b48f22) is red with this patch.
> >   ./build-support/jenkins/build.sh
> > 
> > status = self.run(options, args)
> >   File 
> > "/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/commands/install.py",
> >  line 299, in run
> > requirement_set.prepare_files(finder)
> >   File 
> > "/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/req/req_set.py",
> >  line 359, in prepare_files
> > ignore_dependencies=self.ignore_dependencies))
> >   File 
> > "/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/req/req_set.py",
> >  line 576, in _prepare_file
> > session=self.session, hashes=hashes)
> >   File 
> > "/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/download.py",
> >  line 809, in unpack_url
> > hashes=hashes
> >   File 
> > "/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/download.py",
> >  line 648, in unpack_http_url
> > hashes)
> >   File 
> > "/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/download.py",
> >  line 841, in _download_http_url
> > stream=True,
> >   File 
> > "/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/_vendor/requests/sessions.py",
> >  line 480, in get
> > return self.request('GET', url, **kwargs)
> >   File 
> > "/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/download.py",
> >  line 377, in request
> > return super(PipSession, self).request(method, url, *args, **kwargs)
> >   File 
> > "/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/_vendor/requests/sessions.py",
> >  line 468, in request
> > resp = self.send(prep, **send_kwargs)
> >   File 
> > "/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/_vendor/requests/sessions.py",
> >  line 576, in send
> > r = adapter.send(request, **kwargs)
> >   File 
> > "/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/_vendor/cachecontrol/adapter.py",
> >  line 46, in send
> > resp = super(CacheControlAdapter, self).send(request, **kw)
> >   File 
> > "/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/_vendor/requests/adapters.py",
> >  line 447, in send
> > raise SSLError(e, request=request)
> > SSLError: [Errno 185090050] _ssl.c:344: error:0B084002:x509 certificate 
> > routines:X509_load_cert_crl_file:system lib
> > 
> > ...Installing setuptools, pip, wheel...done.
> > Traceback (most recent call last):
> >   File 
> > "/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv.py",
> >  line 2284, in 
> > main()
> >   File 
> > "/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv.py",
> >  line 703, in main
> > symlink=options.symlink)
> >   File 
> > "/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv.py",
> >  line 904, in create_environment
> > download=download,
> >   File 
> > "/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv.py",
> >  line 861, in install_wheel
> > call_subprocess(cmd, show_stdout=False, extra_env=env)
> >   File 
> > "/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv.py",
> >  line 781, in call_subprocess
> > % (cmd_desc, proc.returncode))
> > OSError: Command /home/jenkins/jenkin...s.venv/bin/python2.7 -c "import 
> > sys, pip; sys...d\"] + sys.argv[1:]))" setuptools pip wheel failed with 
> > error code 2
> > 
> > 
> > I will refresh this build result if you post a review containing 
> > "@ReviewBot retry"

This error is due to a buggy virtualenv version. We should update to 14.0.6 
https://virtualenv.pypa.io/en/latest/changes.html#id1


- Stephan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43567/#review119188
---


On Feb. 14, 2016, 9:35 p.m., Stephan Erb wrote:
> 
> ---

Re: Review Request 43567: Always close Deflater/Inflater streams

2016-02-14 Thread Bill Farner 


> On Feb. 14, 2016, 12:37 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java, line 164
> > 
> >
> > Note: this `close` cannot be moved to `finally` and cannot be replaced 
> > by a `flush`. It is mandatory at this position.

I presume this is because closing has a flush effect that renders `outBytes` 
complete?  A one-line comment to that effect would go a long way.

However, can you additionally do the `close()` in a finally without causing 
harm? (and omit from the `catch` branch)


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43567/#review119187
---


On Feb. 14, 2016, 12:35 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43567/
> ---
> 
> (Updated Feb. 14, 2016, 12:35 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Closing Deflater/Inflater streams explicitly frees their native memory 
> instantly. Otherwise, the memory will only be freed once the object finalizer 
> runs.
> 
> I got the idea from this article 
> http://www.evanjones.ca/java-native-leak-bug.html. Unfortunately, we cannot 
> use the Java close-with-resource feature, as `TTransport` in our current 
> Thrift version does not implement `Closable`.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
> 6b65c0f6cb60b98ff352c4c5d5fed38d52b4b062 
> 
> Diff: https://reviews.apache.org/r/43567/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 43567: Always close Deflater/Inflater streams

2016-02-14 Thread Aurora ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43567/#review119188
---



Master (2b48f22) is red with this patch.
  ./build-support/jenkins/build.sh

status = self.run(options, args)
  File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/commands/install.py",
 line 299, in run
requirement_set.prepare_files(finder)
  File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/req/req_set.py",
 line 359, in prepare_files
ignore_dependencies=self.ignore_dependencies))
  File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/req/req_set.py",
 line 576, in _prepare_file
session=self.session, hashes=hashes)
  File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/download.py",
 line 809, in unpack_url
hashes=hashes
  File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/download.py",
 line 648, in unpack_http_url
hashes)
  File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/download.py",
 line 841, in _download_http_url
stream=True,
  File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/_vendor/requests/sessions.py",
 line 480, in get
return self.request('GET', url, **kwargs)
  File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/download.py",
 line 377, in request
return super(PipSession, self).request(method, url, *args, **kwargs)
  File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/_vendor/requests/sessions.py",
 line 468, in request
resp = self.send(prep, **send_kwargs)
  File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/_vendor/requests/sessions.py",
 line 576, in send
r = adapter.send(request, **kwargs)
  File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/_vendor/cachecontrol/adapter.py",
 line 46, in send
resp = super(CacheControlAdapter, self).send(request, **kw)
  File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl/pip/_vendor/requests/adapters.py",
 line 447, in send
raise SSLError(e, request=request)
SSLError: [Errno 185090050] _ssl.c:344: error:0B084002:x509 certificate 
routines:X509_load_cert_crl_file:system lib

...Installing setuptools, pip, wheel...done.
Traceback (most recent call last):
  File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv.py",
 line 2284, in 
main()
  File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv.py",
 line 703, in main
symlink=options.symlink)
  File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv.py",
 line 904, in create_environment
download=download,
  File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv.py",
 line 861, in install_wheel
call_subprocess(cmd, show_stdout=False, extra_env=env)
  File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.5/virtualenv.py",
 line 781, in call_subprocess
% (cmd_desc, proc.returncode))
OSError: Command /home/jenkins/jenkin...s.venv/bin/python2.7 -c "import sys, 
pip; sys...d\"] + sys.argv[1:]))" setuptools pip wheel failed with error code 2


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Feb. 14, 2016, 8:35 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43567/
> ---
> 
> (Updated Feb. 14, 2016, 8:35 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Closing Deflater/Inflater streams explicitly frees their native memory 
> instantly. Otherwise, the memory will only be freed once the object fina

Re: Review Request 43567: Always close Deflater/Inflater streams

2016-02-14 Thread Stephan Erb 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43567/#review119187
---




src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java (line 162)


Note: this `close` cannot be moved to `finally` and cannot be replaced by a 
`flush`. It is mandatory at this position.


- Stephan Erb


On Feb. 14, 2016, 9:35 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43567/
> ---
> 
> (Updated Feb. 14, 2016, 9:35 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Closing Deflater/Inflater streams explicitly frees their native memory 
> instantly. Otherwise, the memory will only be freed once the object finalizer 
> runs.
> 
> I got the idea from this article 
> http://www.evanjones.ca/java-native-leak-bug.html. Unfortunately, we cannot 
> use the Java close-with-resource feature, as `TTransport` in our current 
> Thrift version does not implement `Closable`.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
> 6b65c0f6cb60b98ff352c4c5d5fed38d52b4b062 
> 
> Diff: https://reviews.apache.org/r/43567/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>