fix: check if handle has been initialized before closing

[wForget]

Which issue does this PR close?

Part of #1553

Rationale for this change

Check that the handle is initialized before closing to avoid native thread panic.

What changes are included in this PR?

How are these changes tested?

before this:

after this:

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 58.38%. Comparing base (f09f8af) to head (0a646fa).
Report is 95 commits behind head on main.

Files with missing lines	Patch %	Lines
...va/org/apache/comet/parquet/NativeBatchReader.java	0.00%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1554      +/-   ##
============================================
+ Coverage     56.12%   58.38%   +2.25%     
- Complexity      976      985       +9     
============================================
  Files           119      122       +3     
  Lines         11743    12217     +474     
  Branches       2251     2280      +29     
============================================
+ Hits           6591     7133     +542     
+ Misses         4012     3951      -61     
+ Partials       1140     1133       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[parthchandra]

And should we set handle to zero after being closed?

[wForget]

Thanks, that makes sense to me.

[parthchandra]

Is it possible to add a unit test for this?

[andygrove]

nit: we refer to both handle and this.handle. I assume they are the same thing but it would be nice to make them consistent

[wForget]

Thanks, changed

[wForget]

Is it possible to add a unit test for this?

Do we need to add a error test case for this obvious anomaly?

[parthchandra]

Is it possible to add a unit test for this?

Do we need to add a error test case for this obvious anomaly?

It's not necessary. I just wanted to see in what condition the NativeBatchReader can be called after close has been called. Just in case there is a fundamental error that we are overlooking.

[wForget]

I just wanted to see in what condition the NativeBatchReader can be called after close has been called.

The scenario I encountered was not NativeBatchReader called after close, but an exception occurred during NativeBatchReader initialization and close was called. You can see some information in the first screenshot of description of #1553

[parthchandra]

I just wanted to see in what condition the NativeBatchReader can be called after close has been called.

The scenario I encountered was not NativeBatchReader called after close, but an exception occurred during NativeBatchReader initialization and close was called. You can see some information in the first screenshot of description of #1553

Ah. Thanks for clarifying