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.

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

CI Win64 - "The parameter is incorrect" / "The handle is invalid" #435

Closed
mratsim opened this issue Sep 11, 2019 · 6 comments
Closed

CI Win64 - "The parameter is incorrect" / "The handle is invalid" #435

mratsim opened this issue Sep 11, 2019 · 6 comments

Comments

@mratsim
Copy link
Contributor

mratsim commented Sep 11, 2019

#415 is blocked on an Windows + 64-bit + -d:release only error in PR #408 #422 #431

Some tests seem to fail for no apparent reason, probably while reading the file. Windows 32-bit does not exhibit this behaviour.

Example: #422 (comment) (https://ci.appveyor.com/project/nimbus/nim-beacon-chain/builds/27293393/job/0b600abqo06snpgi#L3130)

[Suite] Official - Operations - Transfers  [Preset: minimal]
  [OK] [Valid]   success_non_activated
  [OK] [Valid]   success_withdrawable
  [OK] [Valid]   success_active_above_max_effective
  [OK] [Valid]   success_active_above_max_effective_fee
NOT 2019-09-09 23:14:47+00:00 Transfer: incorrect signature              tid=2716
  [OK] [Invalid] invalid_signature
NOT 2019-09-09 23:14:47+00:00 Transfer: only senders who either 1) have never been eligible for activation or 2) are withdrawable or 3) have a balance of MAX_EFFECTIVE_BALANCE after the transfer are valid. tid=2716
  [OK] [Invalid] active_but_transfer_past_effective_balance
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] [Invalid] incorrect_slot
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] [Valid]   transfer_clean
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] [Valid]   transfer_clean_split_to_fee
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] [Invalid] insufficient_balance_for_fee
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] [Invalid] insufficient_balance_for_amount_result_full
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] [Invalid] insufficient_balance_for_combined_result_dust
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] [Invalid] insufficient_balance_for_combined_result_full
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] [Invalid] insufficient_balance_for_combined_big_amount
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] [Invalid] insufficient_balance_for_combined_big_fee
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] [Invalid] insufficient_balance_off_by_1_fee
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] [Invalid] insufficient_balance_off_by_1_amount
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] [Invalid] insufficient_balance_duplicate_as_fee_and_amount
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] [Invalid] no_dust_sender
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] [Invalid] no_dust_recipient
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] [Invalid] non_existent_sender
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] [Invalid] non_existent_recipient
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] [Invalid] invalid_pubkey

Unfortunately, it seems like a non-trivial issue to debug by just relying on the CI as we cannot even reproduce the issue with --opt:size instead of --d:release.

It may even be triggered randomly by the kernel see an unrelated codepath (COM+ objects) that trigger the same error randomly.

At first glance looking through those issues, and given the Nim code it should be related to opening/reading files, and maybe to LFS?

status-im/nim-serialization#9 has been open to rule out running of file descriptors.

@tersec
Copy link
Contributor

tersec commented Sep 11, 2019

It seems reasonable to wrap some of them in

when not 64-bit windows:
   runTest()

or its moral equivalent in terms of maintaining the list of such tests, in such a way as they can be easily found and stripped out later when this issue is resolved.

@jangko
Copy link
Contributor

jangko commented Sep 11, 2019

I can reproduce it locally. I will take a look

@mratsim
Copy link
Contributor Author

mratsim commented Sep 11, 2019

A new one from #436 - https://ci.appveyor.com/project/nimbus/nim-beacon-chain/builds/27346548/job/gki7ew7tkda8ou8w#L242

This BLS test has always been working for month and the next tests work.

[Suite] Official - Shuffling tests [Preset:  [Preset: minimal]
  [OK] Shuffling a sequence of N validators [Preset: minimal]
[Suite] Official - BLS tests
    Unhandled exception: The parameter is incorrect.
 [OSError]
  [FAILED] Private to public key conversion
  [OK] Message signing
  [OK] Aggregating signatures
  [OK] Aggregating public keys

The change was just:

status-im/nim-serialization@e874ae6...master

template loadFile*(Format: distinct type,
                   filename: string,
                   RecordType: distinct type,
                   params: varargs[untyped]): auto =
  mixin init, ReaderType
  var stream = openFile(filename)

  # TODO:
  # Remove this when statement once the following bug is fixed:
  # https://github.com/nim-lang/Nim/issues/9996
  when astToStr(params) != "":
    var reader = init(ReaderType(Format), stream, params)
  else:
    var reader = init(ReaderType(Format), stream)

    reader.readValue(RecordType)
template loadFile*(Format: distinct type,
                   filename: string,
                   RecordType: distinct type,
                   params: varargs[untyped]): auto =
  reader.readValue(RecordType)
  try:
    # TODO:
    # Remove this when statement once the following bug is fixed:
    # https://github.com/nim-lang/Nim/issues/9996
    when astToStr(params) != "":
      var reader = init(ReaderType(Format), stream, params)
    else:
      var reader = init(ReaderType(Format), stream)

    reader.readValue(RecordType)
  finally:
    # TODO: destructors
    if not stream.isNil:
      stream[].close()

mratsim added a commit that referenced this issue Sep 11, 2019
mratsim added a commit that referenced this issue Sep 11, 2019
tersec pushed a commit that referenced this issue Sep 11, 2019
)

* Prepare test suite for transfers

* split API process_transfer / processTransfers

* Add range checks on transfer

* Fix invalid transfer conditions

* don't test on windows 64-bit #435
tersec pushed a commit that referenced this issue Sep 11, 2019
* [Test] Official operations - deposits unit test

* Allow ignoring deposits with invalid signature

* We need stacktraces to debug windows 64 issue #408 (comment)

* fix naming of unit test proc

* Revert "We need stacktraces to debug windows 64 issue #408 (comment)"

This reverts commit 04b8b05.

* skip windows-64 in CI #435

* proposer slashing started to crash as well on win-64 #435
mratsim added a commit that referenced this issue Sep 12, 2019
- reintroduce actually working tests
- skip BLS priv_to_pub (tested in blscurve + broken by #440 and #435)
tersec pushed a commit that referenced this issue Sep 12, 2019
* "industrialize" skipping win64 CI
- reintroduce actually working tests
- skip BLS priv_to_pub (tested in blscurve + broken by #440 and #435)

* missing echo

* try/except OSError doesn't seem to work in unittest so plain ignore + move x64 first for fail fast

* ignore transfers completely in WIn64
mratsim added a commit that referenced this issue Sep 12, 2019
* Bump nim-serialization to include status-im/nim-serialization#10 and maybe solve #435

* bump again - status-im/nim-serialization#12

* fix null pointer dereference if stream is empty: status-im/nim-serialization#13

* pull status-im/nim-serialization#15, hopefully the last bump
mratsim added a commit that referenced this issue Sep 12, 2019
@mratsim mratsim changed the title CI Win64 - "The parameter is incorrect" CI Win64 - "The parameter is incorrect" / "The handle is invalid" Sep 12, 2019
@mratsim
Copy link
Contributor Author

mratsim commented Sep 12, 2019

We got a new one: https://ci.appveyor.com/project/nimbus/nim-beacon-chain/build/job/tfvhv77u93mkf88y#L3212 for PR #442

[Suite] Official - Sanity - Blocks  [Preset: mainnet]
  [OK] [Invalid] Previous slot block transition (prev_slot_block_transition)
  [OK] [Valid]   Same slot block transition (same_slot_block_transition)
  [OK] [Valid]   Empty block transition (empty_block_transition)
  [OK] [Valid]   Skipped Slots (skipped_slots)
  [OK] [Valid]   Empty epoch transition (empty_epoch_transition)
    Unhandled exception: The handle is invalid.
 [OSError]
  [FAILED] [Valid]   Empty epoch transition not finalizing (empty_epoch_transition_not_finalizing)
  [OK] [Valid]   Proposer slashing (proposer_slashing)
  [OK] [Valid]   Attester slashing (attester_slashing)
  [OK] [Valid]   Deposit top up (deposit_top_up)
  [OK] [Valid]   Voluntary exit (voluntary_exit)
  [OK] [Valid]   Balance-driven status transitions (balance_driven_status_transitions)
  [OK] [Valid]   Historical batch (historical_batch)
    Unhandled exception: The handle is invalid.
 [OSError]
  [FAILED] [Valid]   ETH1 data votes consensus (eth1_data_votes_consensus)
    Unhandled exception: The handle is invalid.
 [OSError]
  [FAILED] [Valid]   ETH1 data votes no consensus (eth1_data_votes_no_consensus)

mratsim added a commit that referenced this issue Sep 12, 2019
* enable all sanity slots tests

* enable more tests in sanity blocks

* win64 strikes again #435

* workaround invalid handle in win64 CI

* empty_epoch_transition_not_finalizing only valid in minimal preset

* sanity block ETH1 data votes are also minimal only
@jangko
Copy link
Contributor

jangko commented Sep 13, 2019

I have found our source of bug. see nim-lang/Nim#12186

@mratsim
Copy link
Contributor Author

mratsim commented Nov 1, 2019

closed by #513

@mratsim mratsim closed this as completed Nov 1, 2019
Tomi-3-0 pushed a commit to Tomi-3-0/nimbus-eth2 that referenced this issue Aug 2, 2024
)

* Bump nim-serialization to include status-im/nim-serialization#10 and maybe solve status-im#435

* bump again - status-im/nim-serialization#12

* fix null pointer dereference if stream is empty: status-im/nim-serialization#13

* pull status-im/nim-serialization#15, hopefully the last bump
Tomi-3-0 pushed a commit to Tomi-3-0/nimbus-eth2 that referenced this issue Aug 15, 2024
)

* Bump nim-serialization to include status-im/nim-serialization#10 and maybe solve status-im#435

* bump again - status-im/nim-serialization#12

* fix null pointer dereference if stream is empty: status-im/nim-serialization#13

* pull status-im/nim-serialization#15, hopefully the last bump
Tomi-3-0 pushed a commit to Tomi-3-0/nimbus-eth2 that referenced this issue Aug 21, 2024
)

* Bump nim-serialization to include status-im/nim-serialization#10 and maybe solve status-im#435

* bump again - status-im/nim-serialization#12

* fix null pointer dereference if stream is empty: status-im/nim-serialization#13

* pull status-im/nim-serialization#15, hopefully the last bump
Tomi-3-0 pushed a commit to Tomi-3-0/nimbus-eth2 that referenced this issue Aug 21, 2024
)

* Bump nim-serialization to include status-im/nim-serialization#10 and maybe solve status-im#435

* bump again - status-im/nim-serialization#12

* fix null pointer dereference if stream is empty: status-im/nim-serialization#13

* pull status-im/nim-serialization#15, hopefully the last bump
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants