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

[Bug] JVM crashes when attempting to garbage collect a session that wasn't properly initialized #139

Closed
jonathan-hepp opened this issue Jul 25, 2024 · 1 comment
Closed

[Bug] JVM crashes when attempting to garbage collect a session that wasn't properly initialized #139

jonathan-hepp opened this issue Jul 25, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@jonathan-hepp
Copy link
Contributor

jonathan-hepp commented Jul 25, 2024 •
edited
Loading

Describe the bug

During finalization of the Session object, close() is called on a JNISession for which the open() method did not succeed, resulting in a JVM crash

To reproduce

  1. Attempt to open a Session with invalid credentials or any invalid config that results in a SessionException
  2. Wait for GC to run
  3. JVM crashes with a SIGSEGV

System info

  • Platform: Linux 6.5.0-44-generic, Ubuntu 22.04.1, x86_64
  • Zehoh version: 0.11.0
@jonathan-hepp jonathan-hepp added the bug Something isn't working label Jul 25, 2024
jonathan-hepp added a commit to jonathan-hepp/zenoh-kotlin that referenced this issue Jul 25, 2024
@jonathan-hepp
Fix JVM crash when finalizing a session that wasn't properly initiali…
129bdbd 
…zed (eclipse-zenoh#139)
gabrik pushed a commit that referenced this issue Aug 1, 2024
@jonathan-hepp
Fix JVM crash when finalizing a session that wasn't properly initiali…
2690590 
…zed (#139) (#140)
DariusIMP pushed a commit to ZettaScaleLabs/zenoh-kotlin that referenced this issue Aug 5, 2024
@jonathan-hepp @DariusIMP
Fix JVM crash when finalizing a session that wasn't properly initiali…
675d7cf 
…zed (eclipse-zenoh#139) (eclipse-zenoh#140)
OlivierHecart pushed a commit that referenced this issue Aug 5, 2024
@DariusIMP @eclipse-zenoh-bot
@fuzzypixelz @kydos @diogomatsubara @zettascale-bot @jonathan-hepp
Merge 'main' into 'dev/1.0.0' (#153)
2600933 
* build: Sync  with eclipse-zenoh/zenoh@9d09742 from 2024-06-11 (#109)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@ed6c636 from 2024-06-12 (#112)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@8160b01 from 2024-06-13 (#115)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* Enable releasing from any branch (#114)

* build: Sync  with eclipse-zenoh/zenoh@7adad94 from 2024-06-14 (#117)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@93f93d2 from 2024-06-17 (#118)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@2500e5a from 2024-06-20 (#119)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@869ace6 from 2024-07-02 (#120)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@b93ca84 from 2024-07-03 (#121)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* Enabling snapshot publications for dev/1.0.0 on the CI

* fix: wrong syntax in ci workflow (#124)

* build: Sync  with eclipse-zenoh/zenoh@b3e42ce from 2024-07-08 (#126)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* Update main release workflow (#133)

* Using 'android' property to enable/disable Android configuration (#127)

* Enabling snapshot publications for dev/1.0.0

* refactor(gradle): using 'android' property to enable/disable Android configuration

Examples:
- `gradle build -Pandroid=true` enables build with android
- `gradle build` won't consider any android configuration
- `gradle publishAndroidReleaseToMavenLocal -Pandroid=true` publishes to maven local,
	without `-Pandroid=true` it won't work.

* refactor(gradle): updating publication workflows

* refactor(gradle): updating readme instructions

---------

Co-authored-by: zettascale-bot <[email protected]>

* fix(local publication): triggering compilation of zenoh jni when publishing jvm to maven local (#128)

* Update actions (#130)

* chore: Update setup-java to v4

* chore: update gradle action to newer version

* chore: update gradle/wrapper-validation-action@v3

---------

Co-authored-by: Darius Maitia <[email protected]>
Co-authored-by: zettascale-bot <[email protected]>

* fix: Update gradle actions to newer versions (#134)

* fix: Update buildDir usage for gradle 8

* Update gradle wrapper action to newer version

* calling gradle action via arguments is deprecated

* fix: remove unnecessary call to buildDir()

* fix: add -Pandroid to gradle dokkaHtml call

Otherwise the docs won't build

* Fix documentation publication error on CI (#136)

* build: Sync  with eclipse-zenoh/zenoh@0a969cb from 2024-07-25 (#141)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@e587aa9 from 2024-07-26 (#144)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@2d88c7b from 2024-07-29 (#147)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* Fix JVM crash when finalizing a session that wasn't properly initialized (#139) (#140)

* Fix snapshot publications on the CI (#149)

* Fix ZPubThr example imports (#142)

---------

Co-authored-by: eclipse-zenoh-bot <[email protected]>
Co-authored-by: eclipse-zenoh-bot <[email protected]>
Co-authored-by: Mahmoud Mazouz <[email protected]>
Co-authored-by: kydos <[email protected]>
Co-authored-by: Diogo Matsubara <[email protected]>
Co-authored-by: zettascale-bot <[email protected]>
Co-authored-by: Jonathan Hepp <[email protected]>
DariusIMP added a commit to ZettaScaleLabs/zenoh-kotlin that referenced this issue Aug 16, 2024
@DariusIMP @eclipse-zenoh-bot
@fuzzypixelz @kydos @diogomatsubara @zettascale-bot @jonathan-hepp
Merge 'main' into 'dev/1.0.0' (eclipse-zenoh#153)
70a6d6f 
* build: Sync  with eclipse-zenoh/zenoh@9d09742 from 2024-06-11 (eclipse-zenoh#109)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@ed6c636 from 2024-06-12 (eclipse-zenoh#112)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@8160b01 from 2024-06-13 (eclipse-zenoh#115)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* Enable releasing from any branch (eclipse-zenoh#114)

* build: Sync  with eclipse-zenoh/zenoh@7adad94 from 2024-06-14 (eclipse-zenoh#117)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@93f93d2 from 2024-06-17 (eclipse-zenoh#118)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@2500e5a from 2024-06-20 (eclipse-zenoh#119)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@869ace6 from 2024-07-02 (eclipse-zenoh#120)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@b93ca84 from 2024-07-03 (eclipse-zenoh#121)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* Enabling snapshot publications for dev/1.0.0 on the CI

* fix: wrong syntax in ci workflow (eclipse-zenoh#124)

* build: Sync  with eclipse-zenoh/zenoh@b3e42ce from 2024-07-08 (eclipse-zenoh#126)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* Update main release workflow (eclipse-zenoh#133)

* Using 'android' property to enable/disable Android configuration (eclipse-zenoh#127)

* Enabling snapshot publications for dev/1.0.0

* refactor(gradle): using 'android' property to enable/disable Android configuration

Examples:
- `gradle build -Pandroid=true` enables build with android
- `gradle build` won't consider any android configuration
- `gradle publishAndroidReleaseToMavenLocal -Pandroid=true` publishes to maven local,
	without `-Pandroid=true` it won't work.

* refactor(gradle): updating publication workflows

* refactor(gradle): updating readme instructions

---------

Co-authored-by: zettascale-bot <[email protected]>

* fix(local publication): triggering compilation of zenoh jni when publishing jvm to maven local (eclipse-zenoh#128)

* Update actions (eclipse-zenoh#130)

* chore: Update setup-java to v4

* chore: update gradle action to newer version

* chore: update gradle/wrapper-validation-action@v3

---------

Co-authored-by: Darius Maitia <[email protected]>
Co-authored-by: zettascale-bot <[email protected]>

* fix: Update gradle actions to newer versions (eclipse-zenoh#134)

* fix: Update buildDir usage for gradle 8

* Update gradle wrapper action to newer version

* calling gradle action via arguments is deprecated

* fix: remove unnecessary call to buildDir()

* fix: add -Pandroid to gradle dokkaHtml call

Otherwise the docs won't build

* Fix documentation publication error on CI (eclipse-zenoh#136)

* build: Sync  with eclipse-zenoh/zenoh@0a969cb from 2024-07-25 (eclipse-zenoh#141)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@e587aa9 from 2024-07-26 (eclipse-zenoh#144)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@2d88c7b from 2024-07-29 (eclipse-zenoh#147)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* Fix JVM crash when finalizing a session that wasn't properly initialized (eclipse-zenoh#139) (eclipse-zenoh#140)

* Fix snapshot publications on the CI (eclipse-zenoh#149)

* Fix ZPubThr example imports (eclipse-zenoh#142)

---------

Co-authored-by: eclipse-zenoh-bot <[email protected]>
Co-authored-by: eclipse-zenoh-bot <[email protected]>
Co-authored-by: Mahmoud Mazouz <[email protected]>
Co-authored-by: kydos <[email protected]>
Co-authored-by: Diogo Matsubara <[email protected]>
Co-authored-by: zettascale-bot <[email protected]>
Co-authored-by: Jonathan Hepp <[email protected]>
Mallets pushed a commit that referenced this issue Aug 19, 2024
@DariusIMP @eclipse-zenoh-bot
@fuzzypixelz @kydos @diogomatsubara @zettascale-bot @jonathan-hepp @oteffahi
Api alignment (#146)
c6aff70 
* feat(api alignment): replacing res() with wait()

* feat(api alignment): replacing `withValue(...)` with `.payload(...)` on get queries

* feat(api alignment): making 'value' attribute internal for Query, offering 'payload' and 'encoding' accessors instead.

* feat(api alignment): using ZBytes for Value instead of ByteArray

* feat(api alignment): deserialization for String and Int

* feat(api alignment): deserialization for Byte, Short & Long

* feat(api alignment): deserialization for Float & Double

* feat(api alignment): deserialization

* feat(api alignment): serialization - wip

* feat(api alignment): serialization - wip 2

* feat(api alignment): custom deserializers - wip

* feat(api_alignment): zbytes jni refactor

* feat(api alignment): fixing attachment api

* feat(api alignment): ZBytes example + custom serialization improvements

* feat(api_alignment): serializable maps

* feat(api alignment): serialization refactor

* feat(api alignment): deserialization refactor

* rebase fix

* feat(api alignment): deserialization refactor

* feat(api alignment): splitting Serializable interface into Serializable and Deserializable.

* feat(api alignment): marking DeserializationUtils with @PublishedApi tag

* feat(api alignment): fix clippy errors

* feat(api alignment): kdoc comment for ZBytes + removing Serializable for ZBytes

* feat(api alignment): fix imports

* feat(api alignment): Removing IntoZBytes interface (in favor of Serializable) + moving Into ZBytes extension functions under ZBytes.kt

* feat(api alignment): deserialization refactor

* feat(api alignment): moving Serializable and Deserializable under their own file.

* feat(api alignment): refactoring ZBytesTest.kt

* feat(api alignment): refactoring ZBytes and ZBytesTest

* feat(default args): subscriber - wip

* feat(default args): publisher

* feat(default args): subscriber - updating documentation

* feat(default args): key expressions - removing resolvables

* feat(default args): queryable

* feat(default args): get query - WIP

* feat(default args): put operation - WIP

* feat(default args): delete operation - WIP

* feat(default args): fixing examples

* feat(default args): changing receiver attribute from Queryable, Get and Subscriber to be non optional.

* fix(selector): setting selector parameters as a nullable argument instead of putting an empty string.

* fix(selector): setting selector parameters as a nullable argument instead of putting an empty string.

* feat(default arguments): replacing reply builders with default args

* feat(default arguments): removing QoS builder

* feat(default arguments): making ChannelHandler internal

* feat(default arguments): removing the Get class (unused after changes)

* feat(default arguments): making Encoding and QoS data classes

* feat(default arguments): making Sample a data class

* feat(default arguments): updating KDoc for session.get .

* feat(default arguments): updating KDoc for session.declareQueryable

* feat(default arguments): updating KDoc for session.put and session.delete

* feat(default arguments): updating KDoc for Reply, converting Reply.Success, Reply.Error & Reply.Delete into data classes.

* Merge 'main' into 'dev/1.0.0' (#153)

* build: Sync  with eclipse-zenoh/zenoh@9d09742 from 2024-06-11 (#109)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@ed6c636 from 2024-06-12 (#112)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@8160b01 from 2024-06-13 (#115)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* Enable releasing from any branch (#114)

* build: Sync  with eclipse-zenoh/zenoh@7adad94 from 2024-06-14 (#117)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@93f93d2 from 2024-06-17 (#118)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@2500e5a from 2024-06-20 (#119)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@869ace6 from 2024-07-02 (#120)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@b93ca84 from 2024-07-03 (#121)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* Enabling snapshot publications for dev/1.0.0 on the CI

* fix: wrong syntax in ci workflow (#124)

* build: Sync  with eclipse-zenoh/zenoh@b3e42ce from 2024-07-08 (#126)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* Update main release workflow (#133)

* Using 'android' property to enable/disable Android configuration (#127)

* Enabling snapshot publications for dev/1.0.0

* refactor(gradle): using 'android' property to enable/disable Android configuration

Examples:
- `gradle build -Pandroid=true` enables build with android
- `gradle build` won't consider any android configuration
- `gradle publishAndroidReleaseToMavenLocal -Pandroid=true` publishes to maven local,
	without `-Pandroid=true` it won't work.

* refactor(gradle): updating publication workflows

* refactor(gradle): updating readme instructions

---------

Co-authored-by: zettascale-bot <[email protected]>

* fix(local publication): triggering compilation of zenoh jni when publishing jvm to maven local (#128)

* Update actions (#130)

* chore: Update setup-java to v4

* chore: update gradle action to newer version

* chore: update gradle/wrapper-validation-action@v3

---------

Co-authored-by: Darius Maitia <[email protected]>
Co-authored-by: zettascale-bot <[email protected]>

* fix: Update gradle actions to newer versions (#134)

* fix: Update buildDir usage for gradle 8

* Update gradle wrapper action to newer version

* calling gradle action via arguments is deprecated

* fix: remove unnecessary call to buildDir()

* fix: add -Pandroid to gradle dokkaHtml call

Otherwise the docs won't build

* Fix documentation publication error on CI (#136)

* build: Sync  with eclipse-zenoh/zenoh@0a969cb from 2024-07-25 (#141)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@e587aa9 from 2024-07-26 (#144)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@2d88c7b from 2024-07-29 (#147)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* Fix JVM crash when finalizing a session that wasn't properly initialized (#139) (#140)

* Fix snapshot publications on the CI (#149)

* Fix ZPubThr example imports (#142)

---------

Co-authored-by: eclipse-zenoh-bot <[email protected]>
Co-authored-by: eclipse-zenoh-bot <[email protected]>
Co-authored-by: Mahmoud Mazouz <[email protected]>
Co-authored-by: kydos <[email protected]>
Co-authored-by: Diogo Matsubara <[email protected]>
Co-authored-by: zettascale-bot <[email protected]>
Co-authored-by: Jonathan Hepp <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@b1e4dba from 2024-08-05 (#155)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@b7d42ef from 2024-08-06 (#156)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* build: Sync  with eclipse-zenoh/zenoh@2d5ab7c from 2024-08-06 (#157)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* Add Query `Consolidation::Auto` to JNI and Kotlin API (#158)

* Add Consolidation::Auto to JNI and Kotlin API

* Use default consolidation in ZGet example

* Change default consolidation mode to AUTO (#159)

* Change default consolidation mode to AUTO

* Implement default function for ConsolidationMode

* build: Sync  with eclipse-zenoh/zenoh@0e2f78a from 2024-08-14 (#166)

Co-authored-by: eclipse-zenoh-bot <[email protected]>

* Optimizing KeyExpr processing (#161)

When processing the internal string representation of a kotlin KeyExpr instance
through JNI, a rust KeyExpr instance was generated through the "try_from" function.
This is unnecessary since the string representation was already validated during the
construction phase of the KeyExpr object on Kotlin.
Therefore, this commit introduces an optimization in which the rust key expression
is created with "from_string_unchecked", since we already guarantee that the string
representation provided is valid and has already been checked.

* feat(api alignment): replacing `withValue(...)` with `.payload(...)` on get queries

* feat(api-alignment): removing Resolvable.kt

---------

Co-authored-by: eclipse-zenoh-bot <[email protected]>
Co-authored-by: eclipse-zenoh-bot <[email protected]>
Co-authored-by: Mahmoud Mazouz <[email protected]>
Co-authored-by: kydos <[email protected]>
Co-authored-by: Diogo Matsubara <[email protected]>
Co-authored-by: zettascale-bot <[email protected]>
Co-authored-by: Jonathan Hepp <[email protected]>
Co-authored-by: oteffahi <[email protected]>
@DariusIMP
Copy link
Member

Closing issue since the fix from #140 is already merged on main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jonathan-hepp @DariusIMP