search

sciter-sdk / rust-sciter

Rust bindings for Sciter
https://sciter.com
MIT License
806 stars 75 forks source link

Segfault involving on_data_load and anchor in htm document #59

Open ngirard opened 5 years ago

ngirard commented 5 years ago

This segfault can be easily reproduced by unzipping the minimal project I'm attaching, and typing cargo run.

This minimal application will display

<html>
  <head>
    <title>Segfault when loading main.css and an anchor is present</title>
    <style src="main.css" />
  </head>
<body>
  <h1>Minimal Sciter Application</h1>
  <p><a href="any_url">Sciter SDK</a></p>
</body>
</html>

and the main.css file is send to Sciter using on_data_load in the main.rs file.

Oddly enough:

segfault.zip

pravic commented 5 years ago

Well, the frame variable gets destroyed at the end of its scope and everything goes wrong.

It's clearly a bug, but do you really want to do this instead of using the self.host?

ngirard commented 5 years ago

Hi, thanks for your feedback and big thanks for your commitment to Sciter bindings !

It's clearly a bug, but do you really want to do this instead of using the self.host?

No, apart from reporting the bug, I just wish to do it in the recommended way and move on ; I just had a hard time finding relevant information and examples dealing with on_data_load.

If I'm not mistaken, the only occurrence of code involving on_data_load without using sciter::host::Archive is from a question by Thomas Marlowe on the Sciter forum last November.

If you showed me the proper way to go, I wish I'd make it an example and add it to the examples directory -- what to you think ?

So, how would you do it ? By writing this ?

let host = self.host.upgrade()?;

By the way, Thomas Marlowe's code came incomplete to the forum, but it doesn't seem to use a Weak ref to the sciter::Host, whereas reading the example code (in examples/download.rs) made me think it was mandatory. So how do things stand currently ?

pravic commented 5 years ago

I see, it's a bit confusing now.

Both HostHandler::data_ready and Host::data_ready do the same thing, so it's a matter of convenience, really. You don't need a weak reference just for those purposes (since the self.data_ready() should work).

And now I see the problem in the referenced code:

self.data_ready(pnm.hwnd, &path.to_string_lossy(), &b.as_slice(), Some(pnm.request_id));
return Some(shost::LOAD_RESULT::LOAD_DEFAULT);

vs

self.host.data_ready(&uri, &b.as_slice());
return Some(shost::LOAD_RESULT::LOAD_DEFAULT);

The first attempt specified the request_id parameter, the second one - did not.

This should work, I believe:

self.data_ready(pnm.hwnd, &path.to_string_lossy(), &b.as_slice(), None);

The API is confusing and allows misusage, which is clearly a flaw. I should document it better, to say at least.